Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make nonce optional for OAuth servers that don't support it. #282

Closed
FelipeBuiles opened this issue Sep 3, 2018 · 2 comments
Closed

Make nonce optional for OAuth servers that don't support it. #282

FelipeBuiles opened this issue Sep 3, 2018 · 2 comments
Labels
provider-bug Issues that are affected by bugs or spec non-compliance by an OAuth2 AS / OIDC IDP

Comments

@FelipeBuiles
Copy link

FelipeBuiles commented Sep 3, 2018

AppAuth-iOS worked correctly with AWS Cognito up until its 0.93 release that fixed #4. The issue lies in the inabilty of Cognito to handle the nonce parameter (mentioned in their forums here).
There is a comment on the OIDAuthorizationRequest.h file that mentions something about it being nullable:

    @param nonce String value used to associate a Client session with an ID Token. Can be set to nil
        if not using OpenID Connect, although pure OAuth servers should ignore params they don't
        understand anyway.

The thing is, my team is not using AppAuth-iOS directly but rather trough a wrapper since our project is built with React Native, and we need a way to expose this functionality to that wrapper.

I've opened a PR that implements our fix to this, its also linked to another PR on the wrapper repo.

@WilliamDenniss
Copy link
Member

The right approach here (to avoid sending nonce to a non-compliant provider) is to use the designated initializer of OIDAuthorizationRequest and pass nil for the nonce.

/*! @brief Designated initializer.
@param configuration The service's configuration.
@param clientID The client identifier.
@param scope A scope string per the OAuth2 spec (a space-delimited set of scopes).
@param redirectURL The client's redirect URI.
@param responseType The expected response type.
@param state An opaque value used by the client to maintain state between the request and
callback.
@param nonce String value used to associate a Client session with an ID Token. Can be set to nil
if not using OpenID Connect, although pure OAuth servers should ignore params they don't
understand anyway.
@param codeVerifier The PKCE code verifier. See @c OIDAuthorizationRequest.generateCodeVerifier.
@param codeChallenge The PKCE code challenge, calculated from the code verifier such as with
@c OIDAuthorizationRequest.codeChallengeS256ForVerifier:.
@param codeChallengeMethod The PKCE code challenge method.
::OIDOAuthorizationRequestCodeChallengeMethodS256 when
@c OIDAuthorizationRequest.codeChallengeS256ForVerifier: is used to create the code
challenge.
@param additionalParameters The client's additional authorization parameters.
*/
- (instancetype)
initWithConfiguration:(OIDServiceConfiguration *)configuration
clientId:(NSString *)clientID
clientSecret:(nullable NSString *)clientSecret
scope:(nullable NSString *)scope
redirectURL:(nullable NSURL *)redirectURL
responseType:(NSString *)responseType
state:(nullable NSString *)state
nonce:(nullable NSString *)nonce
codeVerifier:(nullable NSString *)codeVerifier
codeChallenge:(nullable NSString *)codeChallenge
codeChallengeMethod:(nullable NSString *)codeChallengeMethod
additionalParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters
NS_DESIGNATED_INITIALIZER;

@WilliamDenniss WilliamDenniss added the provider-bug Issues that are affected by bugs or spec non-compliance by an OAuth2 AS / OIDC IDP label Sep 3, 2018
@FelipeBuiles
Copy link
Author

FelipeBuiles commented Sep 3, 2018

You're completely right, I'll update my PR on the react-native-app-auth repo to use the correct initializer. Thanks for the guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
provider-bug Issues that are affected by bugs or spec non-compliance by an OAuth2 AS / OIDC IDP
Projects
None yet
Development

No branches or pull requests

2 participants