Make some Credentials properties non-optional [SDK-2900]
#533
Conversation
| @@ -39,8 +39,7 @@ class CredentialsSpec: QuickSpec { | |||
| describe("init from json") { | |||
|
|
|||
| it("should have all tokens and token_type") { | |||
| let credentials = Credentials(json: ["access_token": AccessToken, "token_type": Bearer, "id_token": IdToken, "refresh_token": RefreshToken, "expires_in" : expiresIn, "scope": Scope, "recovery_code": RecoveryCode]) | |||
| expect(credentials).toNot(beNil()) | |||
There was a problem hiding this comment.
When created from JSON, the instance of Credentials cannot be nil. That assertion is superflous.
| expect(credentials.scope).to(beNil()) | ||
| expect(credentials.recoveryCode).to(beNil()) | ||
| } | ||
|
|
||
| it("should have id_token") { |
There was a problem hiding this comment.
All properties (nullable and non-nullable) are tested in the two previous test cases.
| let validatorContext = IDTokenValidatorContext(authentication: authentication, | ||
| issuer: self.issuer, | ||
| leeway: self.leeway, | ||
| maxAge: self.maxAge, | ||
| nonce: self.defaults["nonce"], | ||
| organization: self.organization) | ||
| validateFrontChannelIDToken(idToken: idToken, for: responseType, with: validatorContext) { error in | ||
| validateFrontChannelIDToken(idToken: frontChannelIdToken, for: responseType, with: validatorContext) { error in |
There was a problem hiding this comment.
This method (and everything related to front-channel ID Tokens) will be removed in a future PR, as we'll only support code.
| case cannotDecode | ||
|
|
||
| var errorDescription: String? { | ||
| switch self { | ||
| case .missingToken: return "ID token is required but missing" |
There was a problem hiding this comment.
There's no point in validating the ID Token presence if there will always be an ID Token in the response.
Auth0/CredentialsManager.swift
Outdated
| return jwt.expired | ||
| } | ||
| if credentials.expiresIn < Date() { return true } | ||
| if let jwt = try? decode(jwt: credentials.idToken) { return jwt.expired } |
There was a problem hiding this comment.
@adamjmcgrath should we keep taking into account the ID Token expiration to determine the validity of the credentials?
There was a problem hiding this comment.
Yep 👍 - we should only use the ID Token exp when validating the ID Token
There was a problem hiding this comment.
Ok, I'll remove that check then
Widcket
changed the title
Widcket
changed the title
Credentials properties non-optional [SDK-2900]
Changes
The following properties of the Credentials class were made non-optional, using the empty string ("”) as the default value in the initializer:
accessTokentokenTypeexpiresInidTokenscope,refreshToken, andrecoveryCoderemain optional.Consequently, the ID Token presence check was removed from the ID Token validation logic.
References
The migration guide was updated on #534
Testing
Checklist