Skip to content

Commit

Permalink
Do not check for RT presence on hasValid()
Browse files Browse the repository at this point in the history
  • Loading branch information
Widcket committed Dec 7, 2021
1 parent 82e89d0 commit 5b5e368
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 34 deletions.
4 changes: 1 addition & 3 deletions Auth0/CredentialsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public struct CredentialsManager {
public func hasValid(minTTL: Int = 0) -> Bool {
guard let data = self.storage.getEntry(forKey: self.storeKey),
let credentials = try? NSKeyedUnarchiver.unarchivedObject(ofClass: Credentials.self, from: data) else { return false }
return (!self.hasExpired(credentials) && !self.willExpire(credentials, within: minTTL)) || credentials.refreshToken != nil
return !self.hasExpired(credentials) && !self.willExpire(credentials, within: minTTL)
}

/// Retrieve credentials from keychain and yield new credentials using `refreshToken` if `accessToken` has expired
Expand Down Expand Up @@ -176,7 +176,6 @@ public struct CredentialsManager {
/// - Note: [Auth0 Refresh Tokens Docs](https://auth0.com/docs/security/tokens/refresh-tokens)
#if WEB_AUTH_PLATFORM
public func credentials(withScope scope: String? = nil, minTTL: Int = 0, parameters: [String: Any] = [:], headers: [String: String] = [:], callback: @escaping (CredentialsManagerResult<Credentials>) -> Void) {
guard self.hasValid(minTTL: minTTL) else { return callback(.failure(.noCredentials)) }
if let bioAuth = self.bioAuth {
guard bioAuth.available else {
let error = CredentialsManagerError(code: .biometricsFailed,
Expand All @@ -195,7 +194,6 @@ public struct CredentialsManager {
}
#else
public func credentials(withScope scope: String? = nil, minTTL: Int = 0, parameters: [String: Any] = [:], headers: [String: String] = [:], callback: @escaping (CredentialsManagerResult<Credentials>) -> Void) {
guard self.hasValid(minTTL: minTTL) else { return callback(.failure(.noCredentials)) }
self.retrieveCredentials(withScope: scope, minTTL: minTTL, parameters: parameters, headers: headers, callback: callback)
}
#endif
Expand Down
14 changes: 1 addition & 13 deletions Auth0Tests/CredentialsManagerSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,6 @@ class CredentialsManagerSpec: QuickSpec {
expect(credentialsManager.hasValid()).to(beTrue())
}

it("should have valid credentials when token expired and refresh token present") {
let credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: RefreshToken, expiresIn: Date(timeIntervalSinceNow: -ExpiresIn))
expect(credentialsManager.store(credentials: credentials)).to(beTrue())
expect(credentialsManager.hasValid()).to(beTrue())
}

it("should not have valid credentials when token expired and no refresh token present") {
let credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: nil, expiresIn: Date(timeIntervalSinceNow: -ExpiresIn))
expect(credentialsManager.store(credentials: credentials)).to(beTrue())
Expand All @@ -309,12 +303,6 @@ class CredentialsManagerSpec: QuickSpec {
expect(credentialsManager.hasValid(minTTL: ValidTTL)).to(beTrue())
}

it("should have valid credentials when the ttl is greater than the token lifetime and refresh token present") {
let credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: RefreshToken, expiresIn: Date(timeIntervalSinceNow: ExpiresIn))
expect(credentialsManager.store(credentials: credentials)).to(beTrue())
expect(credentialsManager.hasValid(minTTL: InvalidTTL)).to(beTrue())
}

it("should not have valid credentials when the ttl is greater than the token lifetime and no refresh token present") {
let credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: nil, expiresIn: Date(timeIntervalSinceNow: ExpiresIn))
expect(credentialsManager.store(credentials: credentials)).to(beTrue())
Expand Down Expand Up @@ -396,7 +384,7 @@ class CredentialsManagerSpec: QuickSpec {
}

it("should error when token expired") {
credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: nil, expiresIn: Date(timeIntervalSinceNow: -ExpiresIn))
credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: RefreshToken, expiresIn: Date(timeIntervalSinceNow: -ExpiresIn))
_ = credentialsManager.store(credentials: credentials)
credentialsManager.credentials { result in
expect(result).to(haveCredentialsManagerError(CredentialsManagerError(code: .noCredentials)))
Expand Down
38 changes: 20 additions & 18 deletions V2_MIGRATION_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,14 @@ The default scope value in Web Auth and all the Authentication client methods (e

### Protocols

`AuthTransaction` is no longer public, and the following protocols were removed:
The following protocols were removed:

- `AuthResumable`
- `AuthCancelable`
- `AuthProvider`
- `NativeAuthTransaction`

`AuthResumable` and `AuthCancelable` were subsumed in `AuthTransaction`.
`AuthResumable` and `AuthCancelable` were subsumed in `AuthTransaction`, which is no longer public.

### Type aliases

Expand Down Expand Up @@ -169,21 +169,13 @@ The `a0_url(_:)` method is no longer public.

#### Properties removed

`info: [String: Any]` is no longer public. Use the new subscript to access its values straight from the error; e.g. `error["code"]`.

#### Properties renamed

`description` was renamed to `localizedDescription`, as `AuthenticationError` now conforms to `CustomStringConvertible`.
`info: [String: Any]` is no longer public. Use the new subscript to access its values straight from the error, e.g. `error["code"]`.

### `ManagementError` struct

#### Properties removed

`info: [String: Any]` is no longer public. Use the new subscript to access its values straight from the error; e.g. `error["code"]`.

#### Properties renamed

`description` was renamed to `localizedDescription`, as `ManagementError` now conforms to `CustomStringConvertible`.
`info: [String: Any]` is no longer public. Use the new subscript to access its values straight from the error, e.g. `error["code"]`.

### `WebAuthError` struct

Expand Down Expand Up @@ -384,7 +376,7 @@ class CustomStore: CredentialsStorage {
}
}

let credentialsManager = CredentialsManager(authentication: authentication, storage: CustomStore());
let credentialsManager = CredentialsManager(authentication: authentication, storage: CustomStore())
```

#### `credentials(withScope:minTTL:parameters:callback)`
Expand Down Expand Up @@ -414,7 +406,9 @@ credentialsManager.credentials { result in

## Behavior changes

### `openid` scope enforced on Web Auth
### Web Auth

#### Enforcement of the `openid` scope

If the scopes passed via the Web Auth method `.scope(_:)` do not include the `openid` scope, it will be added automatically.

Expand All @@ -427,10 +421,18 @@ Auth0
}
```

### Credentials expiration on `CredentialsManager`
### Credentials Manager

#### Role of ID Token expiration in credentials validity

The ID Token expiration is no longer used to determine if the credentials are still valid. Only the Access Token expiration is used now.

#### Role of Refresh Token in credentials validity

`hasValid(minTTL:)` will no longer return `true` if a Refresh Token is present. Now, only the Access Token expiration (along with the `minTTL` value) will determine the return value of `hasValid(minTTL:)`.

The `CredentialsManager` class no longer takes into account the ID Token expiration to determine if the credentials are still valid. The only value being considered now is the Access Token expiration.
Note that `hasValid(minTTL:)` is no longer being called in `credentials(withScope:minTTL:parameters:callback:)` _before_ the biometrics authentication. If you were relying on this behavior, you'll need to call `hasValid(minTTL:)` before `credentials(withScope:minTTL:parameters:callback:)` yourself.

### Thread-safety when renewing credentials with the `CredentialsManager`
#### Thread-safety when renewing credentials

The method `credentials(withScope:minTTL:parameters:callback:)` of the `CredentialsManager` class will now execute the credentials renewal serially, to prevent race conditions when Refresh Token Rotation is enabled.
The method `credentials(withScope:minTTL:parameters:callback:)` will now execute the credentials renewal serially, to prevent race conditions when Refresh Token Rotation is enabled.

0 comments on commit 5b5e368

Please sign in to comment.