Implement Authkit Sessions #315
Conversation
Greptile SummaryThis PR successfully implements AuthKit Sessions support for the PHP SDK, bringing it to parity with Node, Python, and Ruby SDKs. The implementation adds secure cookie-based session management with encrypted session storage using the Paragonie Halite library (libsodium). Key Changes:
Bug Fix:
Security:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant App as PHP Application
participant CookieSession
participant UserManagement
participant Encryption as HaliteSessionEncryption
participant API as WorkOS API
Note over Client,API: Initial Authentication & Session Creation
Client->>App: Login request
App->>API: authenticateWithPassword/authenticateWithCode
API-->>App: AuthenticationResponse (access_token, refresh_token)
App->>UserManagement: sealSession(sessionData, cookiePassword)
UserManagement->>Encryption: seal(data, password, ttl)
Encryption->>Encryption: deriveKey(password) via HKDF
Encryption->>Encryption: encrypt with libsodium
Encryption-->>UserManagement: sealed session string
UserManagement-->>App: sealed session
App->>Client: Set encrypted cookie
Note over Client,API: Session Authentication
Client->>App: Request with session cookie
App->>UserManagement: getSessionFromCookie(cookiePassword)
UserManagement->>CookieSession: new CookieSession(userMgmt, sealedSession, password)
App->>CookieSession: authenticate()
CookieSession->>UserManagement: authenticateWithSessionCookie(sealedSession, password)
UserManagement->>Encryption: unseal(sealedSession, password)
Encryption->>Encryption: decrypt with libsodium
Encryption->>Encryption: validate TTL
Encryption-->>UserManagement: session data (access_token, refresh_token)
UserManagement->>API: POST /sessions/authenticate
API-->>UserManagement: SessionAuthenticationSuccessResponse
UserManagement-->>CookieSession: Success/Failure response
CookieSession-->>App: Success/Failure response
App-->>Client: Authenticated request
Note over Client,API: Session Refresh
App->>CookieSession: refresh(options)
CookieSession->>CookieSession: authenticate() to get current session
CookieSession->>UserManagement: authenticateWithRefreshToken(clientId, refreshToken, ip, ua, orgId)
UserManagement->>API: POST /authenticate with refresh_token
API-->>UserManagement: New access_token & refresh_token
UserManagement-->>CookieSession: AuthenticationResponse
CookieSession->>UserManagement: sealSession(newData, newPassword)
UserManagement->>Encryption: seal(newData, newPassword)
Encryption-->>UserManagement: new sealed session
UserManagement-->>CookieSession: new sealed session
CookieSession-->>App: [SuccessResponse, newSealedSession]
App->>Client: Update cookie with new sealed session
Note over Client,API: Session Logout
App->>CookieSession: getLogoutUrl(returnTo)
CookieSession->>CookieSession: authenticate() to get sessionId
CookieSession->>UserManagement: getLogoutUrl(sessionId, returnTo)
UserManagement-->>CookieSession: logout URL
CookieSession-->>App: logout URL
App->>Client: Redirect to logout URL
Client->>API: GET /sessions/logout
API-->>Client: Redirect to returnTo URL
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
birdcar
force-pushed
the
birdcar/session-implementation
branch
from
8ea745b to
c80a0ed
Compare
birdcar
force-pushed
the
birdcar/session-implementation
branch
from
c80a0ed to
19d649b
Compare
|
This doesn't have to wait, but it would likely be beneficial to wait until #316 merges so I can update the new |
dandorman
left a comment
dandorman
left a comment
There was a problem hiding this comment.
This seems good to me, outside of a couple nits. (And i am woefully out of date on my PHP knowledge, so take anything i say with a grain of salt.)
The code looks good, Halite seems like a reasonable choice for an encryption library. I think that is my sole caveat with this PR. It does indeed bring the PHP SDK closer to how the others operate, but that includes the sealed session stuff i think we're hoping to deprecate. Most frameworks provide their own way to encrypt and/or sign cookies, there's little reason for us to add our own layer. Additionally, this creates another island of un-interoperability, where sessions sealed in one platform can't easily be unsealed in another. (cc @cmatheson , who has more thoughts/knowledge on the matter)
Would it be possible to make the sealed-session stuff optional? I don't want to overburden you, but @nicknisi has done some excellent work at paving the way to make session backing more plugin in the JS world.
But if we're not interested in pursuing any of that now, let me know. I don't want to hold this up if we'd rather get parity.
| return Resource\SessionAuthenticationSuccessResponse::constructFromResponse($response); | ||
| } catch (\Exception $e) { | ||
| return new Resource\SessionAuthenticationFailureResponse( | ||
| Resource\SessionAuthenticationFailureResponse::REASON_INVALID_SESSION_COOKIE |
There was a problem hiding this comment.
i think that, e.g., the refresh request could fail for other reasons—at least the HTTP call. Might be worth making the try/catch block more tightly around the session unsealing?
| return [$successResponse, $newSealedSession]; | ||
| } catch (\Exception $e) { | ||
| $failureResponse = new SessionAuthenticationFailureResponse( | ||
| SessionAuthenticationFailureResponse::REASON_INVALID_JWT |
There was a problem hiding this comment.
i think this could fail for reasons besides an invalid JWT. Is it worth tightening the scope of the try/catch block here?
Description
This PR implements AuthKit Sessions support for the PHP SDK, bringing it to parity with the Node, Python, and Ruby sdks.
Documentation
Does this require changes to the WorkOS Docs? E.g. the API Reference or code snippets need updates.
If yes, link a related docs PR and add a docs maintainer as a reviewer. Their approval is required.