vamsii777 / vapor-oauth

OAuth2 Provider Library for Vapor
MIT License
3 stars 2 forks source link

Implement PKCE for Vapor OAuth #1

Closed vamsii777 closed 9 months ago

vamsii777 commented 9 months ago

This pull request introduces support for Proof Key for Code Exchange (PKCE) parameters to enhance the security of OAuth implementation in Vapor. PKCE is a crucial security feature when dealing with public clients, such as mobile and single-page applications.

Changes Made:

  1. Added PKCE parameters to the AuthorizationRequestObject to ensure secure code exchange.
  2. Updated the Vapor dependency version to maintain compatibility with the latest features.
  3. Added PKCE parameters to the OAuthCode class, enabling PKCE support for code exchange.
  4. Refactored the AuthorizationRequestObject initializer signature to accommodate PKCE parameters.
  5. Updated the CodeManager protocol to include methods for handling PKCE parameters.
  6. Implemented PKCE validation for the code challenge method, ensuring code integrity.
  7. Updated the EmptyCodeManager to include PKCE parameters, enhancing code management.
  8. Added token generation and retrieval methods to the TokenManager protocol, necessary for PKCE.
  9. Included PKCE parameters in the AuthorizePostRequest to support PKCE for OAuth.
  10. Imported CryptoKit and included the code_verifier parameter for PKCE code generation and validation.

These changes will greatly enhance the security of OAuth implementation in Vapor, making it more robust and resilient to potential security threats.

mynona commented 9 months ago

Thanks a lot. I am happy to see that this will get added.

vamsii777 commented 9 months ago

Noting here that adding test cases is important for OpenID Configuration.

mynona commented 9 months ago

Hi, I am already trying out the PKCE flow. I might have found a bug:

Bildschirmfoto 2023-12-23 um 19 24 11

As you can see, the authorizationRequestObject doesn't return the codeChallenge and the codeChallengeMethod.

Therefore, the customised authorisation handler doesn't receive these values

-----------------------------
MyAuthorizeHandler().handleAuthorizationRequest()
-----------------------------
Authorization request received from the client:
AuthorizationRequestObject(responseType: "code", clientID: "1", redirectURI: http://localhost:8089/callback, scope: ["admin"], state: Optional("ping-pong"), csrfToken: "d491fe9c9c3f0eb59554e101782b131a41f1c407e5e0b6c1aa72377dea106413", codeChallenge: nil, codeChallengeMethod: nil)
-----------------------------

If this is intentional then please excuse this remark. I just try to be helpful.

I was just wondering how it will be possible to persist the codeChallenge and the codeChallengeMethod if it is not sent to the customised AuthorizationHandler.

Most important: Thx that you enhance vapor/oauth. This is like a Christmas present :-) :-) :-)

vamsii777 commented 9 months ago

Hey @mynona, I appreciate you bringing up the issue, and I'm aware of it as well. Thank you for checking out the work in progress, and I'll continue to address it.

vamsii777 commented 9 months ago

Noting here that adding test cases is important for OpenID Configuration.

Moving to Implement OpenID Connect and will be done there.