Allow all PKCE parameters to be overwritten (not just the code verifi…

…er).
lucasrodriguesus · Feb 22, 2016 · 4c590b3 · 4c590b3
1 parent dfb4dc4
commit 4c590b3
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 51 deletions.
diff --git a/Doxyfile b/Doxyfile
@@ -327,7 +327,9 @@ SEARCH_INCLUDES        = YES
 INCLUDE_PATH           = 
 INCLUDE_FILE_PATTERNS  = 
 PREDEFINED             = NS_UNAVAILABLE \
-                         "NS_ENUM(type, name)=enum name"
+                         "NS_ENUM(type, name)=enum name" \
+                         NS_ASSUME_NONNULL_BEGIN \
+                         NS_ASSUME_NONNULL_END
 EXPAND_AS_DEFINED      = 
 SKIP_FUNCTION_MACROS   = YES
 #---------------------------------------------------------------------------

diff --git a/Source/OIDAuthorizationRequest.h b/Source/OIDAuthorizationRequest.h
@@ -27,6 +27,12 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+/*! @brief The @c code_challenge_method  value for the S256 code challenge.
+    @see https://tools.ietf.org/html/rfc7636#section-4.3
+ */
+extern NSString *const OIDOAuthorizationRequestCodeChallengeMethodS256;
+
+
 /*! @class OIDAuthorizationRequest
     @brief Represents an authorization request.
     @see https://tools.ietf.org/html/rfc6749#section-4
@@ -87,33 +93,25 @@ NS_ASSUME_NONNULL_BEGIN
 
 /*! @property codeVerifier
     @brief The PKCE code verifier.
-    @discussion The PKCE codeVerifier (if not nil) is used to generate the @c codeChallenge which is
-        sent in the request. The codeVerifier itself is not included in the authorization request
-        that is sent on the wire, but should be sent in the token exchange request.
+    @remarks code_verifier
+    @discussion The code verifier itself is not included in the authorization request that is sent
+        on the wire, but needs to be in the token exchange request.
         @c OIDAuthorizationResponse.tokenExchangeRequest will create a @c OIDTokenRequest that
         includes this parameter automatically.
     @see https://tools.ietf.org/html/rfc7636#section-4.1
  */
 @property(nonatomic, readonly, nullable) NSString *codeVerifier;
 
 /*! @property codeChallenge
-    @brief The PKCE code_challenge.
+    @brief The PKCE code challenge, derived from #codeVerifier.
     @remarks code_challenge
-    @discussion The PKCE code_challenge derived from the @c codeVerifier per the specification,
-        by base64url encoding (with no padding) the SHA256 of the @c codeVerifier. If
-        @c codeVerifier is set, it will be calculated and sent along with @c codeChallengeMethod
-        in the authorization request.
     @see https://tools.ietf.org/html/rfc7636#section-4.2
  */
 @property(nonatomic, readonly, nullable) NSString *codeChallenge;
 
 /*! @property codeChallengeMethod
-    @brief The PKCE code challenge method.
+    @brief The method used to compute the @c #codeChallenge
     @remarks code_challenge_method
-    @discussion If this request includes @c codeChallenge, this value be "S256", otherwise nil.
-       The PKCE "plain" method is not supported by AppAuth, as iOS is capable of generating a SHA256
-       hash and is mandatory to implement (MTI) for servers who support PKCE. If you need to use
-       "plain" for some reason, it is possible to do manually using the @c #additionalParameters.
     @see https://tools.ietf.org/html/rfc7636#section-4.3
  */
 @property(nonatomic, readonly, nullable) NSString *codeChallengeMethod;
@@ -132,14 +130,16 @@ NS_ASSUME_NONNULL_BEGIN
 - (nullable instancetype)init NS_UNAVAILABLE;
 
 /*! @fn initWithConfiguration:clientId:scopes:redirectURL:responseType:additionalParameters:
-    @brief Creates an authorization request.
+    @brief Creates an authorization request with opinionated defaults (a secure @c state, and
+        PKCE with S256 as the @c code_challenge_method).
     @param configuration The service's configuration.
     @param clientID The client identifier.
     @param scopes An array of scopes to combine into a single scope string per the OAuth2 spec.
     @param redirectURL The client's redirect URI.
     @param responseType The expected response type.
     @param additionalParameters The client's additional authorization parameters.
-    @remarks This convenience initializer generates a state parameter automatically.
+    @remarks This convenience initializer generates a state parameter and PKCE challenges
+        automatically.
  */
 - (nullable instancetype)initWithConfiguration:(OIDServiceConfiguration *)configuration
                 clientId:(NSString *)clientID
@@ -148,16 +148,22 @@ NS_ASSUME_NONNULL_BEGIN
             responseType:(NSString *)responseType
     additionalParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters;
 
-/*! @fn initWithConfiguration:clientId:scope:redirectURL:responseType:state:codeVerifier:additionalParameters:
+/*! @fn initWithConfiguration:clientId:scope:redirectURL:responseType:state:codeVerifier:codeChallenge:codeChallengeMethod:additionalParameters:
     @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 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 codeVerifier The PKCE code verifier.
+    @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.
  */
 - (nullable instancetype)initWithConfiguration:(OIDServiceConfiguration *)configuration
@@ -167,6 +173,8 @@ NS_ASSUME_NONNULL_BEGIN
             responseType:(NSString *)responseType
                    state:(nullable NSString *)state
             codeVerifier:(nullable NSString *)codeVerifier
+           codeChallenge:(nullable NSString *)codeChallenge
+     codeChallengeMethod:(nullable NSString *)codeChallengeMethod
     additionalParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters
     NS_DESIGNATED_INITIALIZER;
 
@@ -183,14 +191,26 @@ NS_ASSUME_NONNULL_BEGIN
     @return The generated state.
     @see https://tools.ietf.org/html/rfc6819#section-5.3.5
  */
-+ (NSString *)generateState;
++ (nullable NSString *)generateState;
 
 /*! @fn generateCodeVerifier
     @brief Constructs a PKCE-compliant code verifier.
     @return The generated code verifier.
     @see https://tools.ietf.org/html/rfc7636#section-4.1
  */
-+ (NSString *)generateCodeVerifier;
++ (nullable NSString *)generateCodeVerifier;
+
+/*! @fn codeChallengeS256ForVerifier:
+    @brief Creates a PKCE S256 codeChallenge from the codeVerifier.
+    @param codeVerifier The code verifier from which the code challenge will be derived.
+    @return The generated code challenge.
+    @details Generate a secure code verifier to pass into this method with
+        @c OIDAuthorizationRequest.generateCodeVerifier. The matching @c #codeChallengeMethod for
+        @c #codeChallenge%s created by this method is
+        ::OIDOAuthorizationRequestCodeChallengeMethodS256.
+    @see https://tools.ietf.org/html/rfc7636#section-4.1
+ */
++ (nullable NSString *)codeChallengeS256ForVerifier:(nullable NSString *)codeVerifier;
 
 @end
 

diff --git a/Source/OIDAuthorizationRequest.m b/Source/OIDAuthorizationRequest.m
@@ -88,24 +88,17 @@
  */
 static NSUInteger const kCodeVerifierBytes = 32;
 
-/*! @var kPKCEChallengeMethodS256
-    @brief The code_challenge_method used by this library (always S256 since iOS is capable of
-        generating a SHA256 hash easily).
-    @see https://tools.ietf.org/html/rfc7636#section-4.3
- */
-static NSString *const kPKCEChallengeMethodS256 = @"S256";
+NSString *const OIDOAuthorizationRequestCodeChallengeMethodS256 = @"S256";
 
 @implementation OIDAuthorizationRequest
 
 - (instancetype)init
     OID_UNAVAILABLE_USE_INITIALIZER(
         @selector(initWithConfiguration:
                                clientId:
-                                  scope:
+                                 scopes:
                             redirectURL:
                            responseType:
-                                  state:
-                           codeVerifier:
                    additionalParameters:)
     );
 
@@ -116,7 +109,10 @@ - (nullable instancetype)initWithConfiguration:(OIDServiceConfiguration *)config
             responseType:(NSString *)responseType
                    state:(nullable NSString *)state
             codeVerifier:(nullable NSString *)codeVerifier
-    additionalParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters {
+           codeChallenge:(nullable NSString *)codeChallenge
+     codeChallengeMethod:(nullable NSString *)codeChallengeMethod
+    additionalParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters
+{
   self = [super init];
   if (self) {
     _configuration = [configuration copy];
@@ -126,6 +122,9 @@ - (nullable instancetype)initWithConfiguration:(OIDServiceConfiguration *)config
     _responseType = [responseType copy];
     _state = [state copy];
     _codeVerifier = [codeVerifier copy];
+    _codeChallenge = [codeChallenge copy];
+    _codeChallengeMethod = [codeChallengeMethod copy];
+
     _additionalParameters =
         [[NSDictionary alloc] initWithDictionary:additionalParameters copyItems:YES];
   }
@@ -138,13 +137,20 @@ - (nullable instancetype)initWithConfiguration:(OIDServiceConfiguration *)config
              redirectURL:(NSURL *)redirectURL
             responseType:(NSString *)responseType
     additionalParameters:(nullable NSDictionary<NSString *, NSString *> *)additionalParameters {
+
+  // generates PKCE code verifier and challenge
+  NSString *codeVerifier = [[self class] generateCodeVerifier];
+  NSString *codeChallenge = [[self class] codeChallengeS256ForVerifier:codeVerifier];
+
   return [self initWithConfiguration:configuration
                             clientId:clientID
                                scope:[OIDScopeUtilities scopesWithArray:scopes]
                          redirectURL:redirectURL
                         responseType:responseType
                                state:[[self class] generateState]
-                        codeVerifier:[[self class] generateCodeVerifier]
+                        codeVerifier:codeVerifier
+                       codeChallenge:codeChallenge
+                 codeChallengeMethod:OIDOAuthorizationRequestCodeChallengeMethodS256
                 additionalParameters:additionalParameters];
 }
 
@@ -174,6 +180,10 @@ - (instancetype)initWithCoder:(NSCoder *)aDecoder {
   NSURL *redirectURL = [aDecoder decodeObjectOfClass:[NSURL class] forKey:kRedirectURLKey];
   NSString *state = [aDecoder decodeObjectOfClass:[NSString class] forKey:kStateKey];
   NSString *codeVerifier = [aDecoder decodeObjectOfClass:[NSString class] forKey:kCodeVerifierKey];
+  NSString *codeChallenge =
+      [aDecoder decodeObjectOfClass:[NSString class] forKey:kCodeChallengeKey];
+  NSString *codeChallengeMethod =
+      [aDecoder decodeObjectOfClass:[NSString class] forKey:kCodeChallengeMethodKey];
   NSSet *additionalParameterCodingClasses = [NSSet setWithArray:@[
     [NSDictionary class],
     [NSString class]
@@ -189,6 +199,8 @@ - (instancetype)initWithCoder:(NSCoder *)aDecoder {
                         responseType:responseType
                                state:state
                         codeVerifier:codeVerifier
+                       codeChallenge:codeChallenge
+                 codeChallengeMethod:codeChallengeMethod
                 additionalParameters:additionalParameters];
   return self;
 }
@@ -201,6 +213,8 @@ - (void)encodeWithCoder:(NSCoder *)aCoder {
   [aCoder encodeObject:_redirectURL forKey:kRedirectURLKey];
   [aCoder encodeObject:_state forKey:kStateKey];
   [aCoder encodeObject:_codeVerifier forKey:kCodeVerifierKey];
+  [aCoder encodeObject:_codeChallenge forKey:kCodeChallengeKey];
+  [aCoder encodeObject:_codeChallengeMethod forKey:kCodeChallengeMethodKey];
   [aCoder encodeObject:_additionalParameters forKey:kAdditionalParametersKey];
 }
 
@@ -213,37 +227,27 @@ - (NSString *)description {
                                     self.authorizationRequestURL];
 }
 
-#pragma mark - CodeVerifier/state Generation Methods
+#pragma mark - State and PKCE verifier/challenge generation Methods
 
-+ (NSString *)generateCodeVerifier {
++ (nullable NSString *)generateCodeVerifier {
   return [OIDTokenUtilities randomURLSafeStringWithSize:kCodeVerifierBytes];
 }
 
-+ (NSString *)generateState {
++ (nullable NSString *)generateState {
   return [OIDTokenUtilities randomURLSafeStringWithSize:kStateSizeBytes];
 }
 
-#pragma mark - PKCE params
-
-- (NSString *)codeChallenge {
-  if (!_codeVerifier) {
++ (nullable NSString *)codeChallengeS256ForVerifier:(NSString *)codeVerifier {
+  if (!codeVerifier) {
     return nil;
   }
   // generates the code_challenge per spec https://tools.ietf.org/html/rfc7636#section-4.2
   // code_challenge = BASE64URL-ENCODE(SHA256(ASCII(code_verifier)))
   // NB. the ASCII conversion on the code_verifier entropy was done at time of generation.
-  NSData *sha256Verifier = [OIDTokenUtilities sha265:_codeVerifier];
+  NSData *sha256Verifier = [OIDTokenUtilities sha265:codeVerifier];
   return [OIDTokenUtilities encodeBase64urlNoPadding:sha256Verifier];
 }
 
-- (NSString *)codeChallengeMethod {
-  if (!_codeVerifier) {
-    return nil;
-  }
-  return kPKCEChallengeMethodS256;
-}
-
-
 #pragma mark -
 
 - (NSURL *)authorizationRequestURL {
@@ -266,9 +270,11 @@ - (NSURL *)authorizationRequestURL {
   if (_state) {
     [query addParameter:kStateKey value:_state];
   }
-  if (_codeVerifier) {
-    [query addParameter:kCodeChallengeKey value:self.codeChallenge];
-    [query addParameter:kCodeChallengeMethodKey value:self.codeChallengeMethod];
+  if (_codeChallenge) {
+    [query addParameter:kCodeChallengeKey value:_codeChallenge];
+  }
+  if (_codeChallengeMethod) {
+    [query addParameter:kCodeChallengeMethodKey value:_codeChallengeMethod];
   }
 
   // Construct the URL:

diff --git a/UnitTests/OIDAuthorizationRequestTests.m b/UnitTests/OIDAuthorizationRequestTests.m
@@ -141,6 +141,13 @@ allowed characters range (the forward slash "\").
 
 @implementation OIDAuthorizationRequestTests
 
++ (NSString *)codeChallenge {
+  return [OIDAuthorizationRequest codeChallengeS256ForVerifier:kTestCodeVerifier];
+}
++ (NSString *)codeChallengeMethod {
+  return OIDOAuthorizationRequestCodeChallengeMethodS256;
+}
+
 + (OIDAuthorizationRequest *)testInstance {
   NSDictionary *additionalParameters =
       @{ kTestAdditionalParameterKey : kTestAdditionalParameterValue };
@@ -153,6 +160,8 @@ + (OIDAuthorizationRequest *)testInstance {
                   responseType:kTestResponseType
                          state:kTestState
                   codeVerifier:kTestCodeVerifier
+                 codeChallenge:[[self class] codeChallenge]
+           codeChallengeMethod:[[self class] codeChallengeMethod]
           additionalParameters:additionalParameters];
   return request;
 }
@@ -167,6 +176,8 @@ + (OIDAuthorizationRequest *)testInstanceCodeFlow {
                   responseType:OIDResponseTypeCode
                          state:kTestState
                   codeVerifier:kTestCodeVerifier
+                 codeChallenge:[[self class] codeChallenge]
+           codeChallengeMethod:[[self class] codeChallengeMethod]
           additionalParameters:nil];
   return request;
 }
@@ -207,6 +218,8 @@ - (void)testCopying {
   XCTAssertEqualObjects(request.redirectURL, [NSURL URLWithString:kTestRedirectURL]);
   XCTAssertEqualObjects(request.state, kTestState);
   XCTAssertEqualObjects(request.codeVerifier, kTestCodeVerifier);
+  XCTAssertEqualObjects(request.codeChallenge, [[self class] codeChallenge]);
+  XCTAssertEqualObjects(request.codeChallengeMethod, [[self class] codeChallengeMethod]);
   XCTAssertEqualObjects(request.additionalParameters[kTestAdditionalParameterKey],
                         kTestAdditionalParameterValue);
 
@@ -220,6 +233,8 @@ - (void)testCopying {
   XCTAssertEqualObjects(requestCopy.redirectURL, request.redirectURL);
   XCTAssertEqualObjects(requestCopy.state, request.state);
   XCTAssertEqualObjects(requestCopy.codeVerifier, request.codeVerifier);
+  XCTAssertEqualObjects(requestCopy.codeChallenge, request.codeChallenge);
+  XCTAssertEqualObjects(requestCopy.codeChallengeMethod, request.codeChallengeMethod);
   XCTAssertEqualObjects(requestCopy.additionalParameters,
                         request.additionalParameters);
 }
@@ -237,6 +252,8 @@ - (void)testSecureCoding {
   XCTAssertEqualObjects(request.redirectURL, [NSURL URLWithString:kTestRedirectURL]);
   XCTAssertEqualObjects(request.state, kTestState);
   XCTAssertEqualObjects(request.codeVerifier, kTestCodeVerifier);
+  XCTAssertEqualObjects(request.codeChallenge, [[self class] codeChallenge]);
+  XCTAssertEqualObjects(request.codeChallengeMethod, [[self class] codeChallengeMethod]);
   XCTAssertEqualObjects(request.additionalParameters[kTestAdditionalParameterKey],
                         kTestAdditionalParameterValue);
 
@@ -257,6 +274,8 @@ - (void)testSecureCoding {
   XCTAssertEqualObjects(requestCopy.redirectURL, [NSURL URLWithString:kTestRedirectURL]);
   XCTAssertEqualObjects(requestCopy.state, kTestState);
   XCTAssertEqualObjects(requestCopy.codeVerifier, kTestCodeVerifier);
+  XCTAssertEqualObjects(requestCopy.codeChallenge, [[self class] codeChallenge]);
+  XCTAssertEqualObjects(requestCopy.codeChallengeMethod, [[self class] codeChallengeMethod]);
   XCTAssertEqualObjects(requestCopy.additionalParameters[kTestAdditionalParameterKey],
                         kTestAdditionalParameterValue);
 }