-
Notifications
You must be signed in to change notification settings - Fork 200
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
Intermittent Session Dumping from Keychain on Unexpected Error in Amplify Auth #3540
Comments
@ostanik Thanks for opening up the issue. From the logs that you have shared, it don't see anything that should cause session being dumped. What I have observed from the logs is that a logged out user is trying to make Furthermore, Amplify never logs out the user by itself, even if the session has expired. The only occasion session is removed from keychain, is when Amplify determines that there was a change in user pool configuration. To further investigate, I would need more concrete details of session dump that was observed. Either exact flow of the Amplify API calls or verbose logs when the issue occurred. Lastly, the code snippet that you shared should not cause any issues, when retrieving the session any amount of times. |
@harsh62 , Clarifications:
We're also encountering unexpected session expirations, which will be addressed separately. Given these challenges, any further insights or advice would be greatly appreciated as we aim to resolve this perplexing issue. |
Thanks for providing more context. Few more questions based on your answers:
Would you be able to provide more code snippets and surrounding code that could impact how Amplify works? The new information you provide, could may be help me with a direction to investigate the issue you are seeing. |
Code Snippet for Auth handling: nit(_ env: ConfigurationEnvironment) {
Amplify.Logging.logLevel = env == .staging ? .verbose : .none
// This is used only to capture traces of auth events
unsubscribeToken = Amplify.Hub.listen(to: .auth) { payload in
AppLogger.shared.debug("Received \(payload.eventName) event from Amplify: \nData: \(String(describing: payload.data))")
}
}
deinit {
Amplify.Hub.removeListener(unsubscribeToken)
}
/// Start the passwordless authentication process
/// - Parameters:
/// - email: The email address to send the magic link to
/// - authType: The type of authentication to perform
/// > Note: This function will retry at most one time if the first attempt fails with a `invalidState` error.
/// This retry is required in order to "request a new link" feature works properly
func startPasswordlessAuth(email: String, authType: PasswordlessAuthType) -> Future<Void, LoginFeature.RequestLinkError> {
AppLogger.shared.debug("Passwordless Auth started")
return Future { promise in
Task { [weak self] in
var hasRetried = false // Flag to track whether a retry has been attempted
func attemptRequest() async {
do {
guard let self = self else {
AppLogger.shared.debug("Start Passwordless Auth: self is nil")
throw LoginFeature.RequestLinkError.authServiceUnavailable
}
try self.configureAmplifyIfNeeded()
// Forcing email to be lower because the user email was configured to be case sensitive
// and BE is lowering all the email addresses (we hope).
_ = try await self.signin(email: email.lowercased())
AppLogger.shared.debug("Passwordless Auth requested")
promise(.success(()))
} catch let error as AuthError {
if case .invalidState = error, !hasRetried {
AppLogger.shared.debug("Invalid state error occurred, attempting retry")
hasRetried = true
await Amplify.Auth.signOut()
await attemptRequest() // Retry request after sign out
} else {
AppLogger.shared.error("Start Passwordless Auth: \(error.debugDescription)", error: error)
promise(.failure(.other))
}
} catch {
AppLogger.shared.error("Start Passwordless Auth: \(error.localizedDescription)", error: error)
promise(.failure(.other))
}
}
await attemptRequest() // Initial call to the request function
}
}
}
func login(code: String) -> Future<Void, Error> {
AppLogger.shared.debug("Claim login code started")
return Future { promise in
Task { [weak self] in
guard let self else {
AppLogger.shared.debug("Claim login code: self is nil")
return promise(.failure(RequestLinkError.authServiceUnavailable))
}
do {
try self.configureAmplifyIfNeeded()
let signInResult = try await Amplify.Auth.confirmSignIn(challengeResponse: code)
AppLogger.shared.debug("Confirm sign in succeeded. Next step: \(signInResult.nextStep)")
// Fetch session right after login to avoid concurrency issues.
// this is a defensive way of working...Should we remove it?
let session = try await Amplify.Auth.fetchAuthSession()
guard let cognitoTokenProvider = session as? AuthCognitoTokensProvider else {
throw RequestLinkError.other
}
_ = try cognitoTokenProvider.getCognitoTokens().get()
AppLogger.shared.debug("Claim login code success")
promise(.success(()))
} catch let error as AuthError {
AppLogger.shared.error("Claim login code: \(error.debugDescription)", error: error)
promise(.failure(error))
} catch {
AppLogger.shared.error("Claim login code: \(error.localizedDescription)", error: error)
promise(.failure(error))
}
}
}
}
func obtainRenewedCredentialsIfNeeded() async -> Bool {
do {
try self.configureAmplifyIfNeeded()
_ = try await fetchCredentials().eraseToAnyPublisher().async()
AppLogger.shared.debug("Obtain renewed Credentials if needed: success - Has Valid Credentials: \(hasValidCredentials)")
return hasValidCredentials
} catch let error as AuthError {
AppLogger.shared.error("Obtain renewed Credentials if needed: \(error.debugDescription)", error: error)
} catch {
AppLogger.shared.error("Obtain renewed Credentials if needed: \(error.localizedDescription)", error: error)
}
return false
}
func fetchCredentials() -> Future<String, Error> {
self.printKeychainItems()
AppLogger.shared.debug("Fetch Credentials started")
return Future { promise in
Task { [weak self] in
guard let self else {
AppLogger.shared.debug("Fetch Credentials: self is nil")
return promise(.failure(RequestLinkError.authServiceUnavailable))
}
do {
try self.configureAmplifyIfNeeded()
let session = try await Amplify.Auth.fetchAuthSession()
if let cognitoTokenProvider = session as? AuthCognitoTokensProvider {
let tokens = try cognitoTokenProvider.getCognitoTokens().get()
AppLogger.shared.debug("Fetch Credentials success")
self.hasValidCredentials = true
promise(.success((tokens.idToken)))
} else {
throw RequestLinkError.other
}
} catch let error as AuthError {
AppLogger.shared.error("Fetch Credentials failed: \(error.debugDescription)", error: error)
switch error {
case .sessionExpired, .signedOut:
AppLogger.shared.debug("Session expired or user signed out, changing valid credentials to false")
self.hasValidCredentials = false
default: break
}
promise(.failure(error))
} catch let error as ConfigurationError {
AppLogger.shared.error("Fetch credentials failed after retry: \(error.debugDescription)", error: error)
promise(.failure(error))
} catch {
AppLogger.shared.error("Fetch Credentials failed: \(error.localizedDescription)", error: error)
promise(.failure(error))
}
}
}
}
// Change this to be async
func clearCredentials() {
Task { [weak self] in
guard let self else {
AppLogger.shared.debug("Clear Credentials: self is nil")
return
}
AppLogger.shared.debug("Cleaning credentials")
do {
// Performing sign out without configure the SDK will cause a crash.
try self.configureAmplifyIfNeeded()
_ = await Amplify.Auth.signOut()
self.hasValidCredentials = false
} catch let error as ConfigurationError {
AppLogger.shared.error("Sign out error \(error.debugDescription)", error: error)
} catch {
AppLogger.shared.error("Sign out error \(error.localizedDescription)", error: error)
}
}
}
/// This method is used to configure Amplify with the Cognito plugin.
/// Amplify configuration should only be called once otherwise it will thrown an error "amplifyAlreaadyConfigured"
/// But on the catching of this function we are handling this error to return and not rethrows. So it should be safe to call this function multiple times.
private func configureAmplifyIfNeeded() throws {
do {
// Creating plugin network preferences to retry 3 times before fail
let cognitoNetworkPreferences = AWSCognitoNetworkPreferences(
maxRetryCount: 3,
timeoutIntervalForRequest: .seconds(60),
timeoutIntervalForResource: .days(7)
)
try Amplify.add(plugin: AWSCognitoAuthPlugin(networkPreferences: cognitoNetworkPreferences))
let userPoolId = BuildSetting(type: .cognitoUserPoolID).value
let clientId = BuildSetting(type: .cognitoClientID).value
let region = BuildSetting(type: .cognitoRegion).value
let configuration = AmplifyConfiguration(
auth: AuthCategoryConfiguration(
plugins: [
"awsCognitoAuthPlugin": [
"IdentityManager": [
"Default": []
],
"CognitoUserPool": [
"Default": [
"PoolId": .string(userPoolId),
"Region": .string(region),
"AppClientId": .string(clientId)
]
],
"Auth": [
"Default": [
"authenticationFlowType": "CUSTOM_AUTH_WITHOUT_SRP"
]
]
]
]
)
)
try Amplify.configure(configuration)
AppLogger.shared.debug("Amplify is configured")
} catch let error as ConfigurationError {
if case .amplifyAlreadyConfigured = error {
AppLogger.shared.debug("Amplify configuration: amplify already configured skipping")
return
}
// Can we only use the configure amplify method once we are not throwing if already configured?
AppLogger.shared.error("Amplify configuration: \(error.debugDescription)", error: error)
throw error
} catch {
AppLogger.shared.error("Amplify configuration: \(error.localizedDescription)", error: error)
throw error
}
}
private func signin(email: String) async throws -> AuthSignInResult {
let customWithoutSRP = AWSAuthSignInOptions(authFlowType: .customWithoutSRP)
let options = AuthSignInRequest.Options(pluginOptions: customWithoutSRP)
return try await Amplify.Auth.signIn(username: email, options: options)
} If there are specific areas of this code or the authentication flow you'd like more details on, or if there are specific concerns, I'd be happy to provide further explanations or adjust our approach based on your guidance. |
I don't know if this helps but here is one scenario from a real user who was authenticated before but right after an app update, got the error:
|
@harsh62 does these log tell you anything? I am just wandering if you are able to spot any problem. Thanks for your help. |
I don't see anything in the logs other than that there no session. It would need to be verbose logs, which could help us in figuring out what is going on. I also looked at your code, and one thing that popped out was The other thing that I see is that you are building the configuration directly in the code. While it is possible to build a configuration this way, the recommended way would be to use an Lastly, I would like to understand how |
@ostanik |
@harsh62 In case |
We're building amplify configuration at runtime from build settings which depend on selected environment (staging/production). |
|
We cannot confirm this error only happening to users who update the app. We have some reports that this error happened after an update, but as far as we know this error happens randomly when user launch the app. However, we can confirm there was no configuration change recently. |
Thank you - We will investigate and post followup questions here. |
@victorkifer We are not able to reproduce this issue at our end. Would you be able to assist with any additional details that could help us further investigate the issue?
|
Hey @harsh62,
I hope this information helps you further investigate the issue. Please let me know if you have any other questions. |
@ostanik Unfortunately I am still not able to repro and not able to find any obvious problems in the code. At this moment, I would like you to reach out to us on discord (https://discord.com/channels/705853757799399426/1019643921137139772) tagging me (my username |
@harsh62 I can't access the link that you shared. |
@ostanik Can you try https://discord.com/invite/amplify? |
Hello, this issue sounds like something we're also experiencing, are there any news? |
@dandreiolteanu I have been unable to reproduce this issue in a local environment, and also unable to find any obvious code issues with Keychain and State Machine implementation. |
We're also seeing behavior in our app which seems to line up with this issues. I don't have full logs unfortunately, only what I've been able to piece together via analytics. We have the same testers on iOS and Android, and the code is the same across platforms, but we've only ever seen this issue on iOS. The issue appears to happen when calling fetchAuthSession right after the device wakes up, and has been asleep for some time. While that call fails attempting to fetch/refresh the auth token, and with Amplify.Hub.subscribe setup, AuthChannelEventName.SESSION_EXPIRED does not appear to be triggered. The failure to fetchAuthSession reports: |
The only other info I have is a screenshot from a debug build which reports an error of "The network connection was lost." It was attempting to contact a url of "https://cognito-idp.us-west-2.amazonaws.com/". |
I don't know how the internal networking is handled, but is the framework attempting to preflight network connectivity before actually making a request? In general, that goes against the recommended behavior and could lead to an issue where connectivity checking fails because the device radios haven't been powered up yet right after wake. This may also relate to another closed issue that my coworker pointed out: Normally you'd just set waitsForConnectivity and a short timeoutIntervalForResource value for the URLSessionConfiguration and issue any calls without any preflight connectivity checking. Apologies if this has been investigated previously. |
@kparichan Would you be able to open another issue with the relevant details and verbose logs. With the brief description you've give us, your issue seems to be something else, which is different from the session dumping issue that is being discussed here in this one. |
Describe the bug
We are encountering issues with Amplify's session management where several users experience their sessions being removed from the keychain following an unexpected error. This results in a 'signedOut' state error when attempting to fetch the auth session from Amplify. The problem does not occur consistently but has been observed under certain conditions.
Steps To Reproduce
Expected behavior
The auth session should be reliably fetched from the keychain without being inadvertently dumped, allowing for uninterrupted user authentication and session management.
Amplify Framework Version
2.26.4
Amplify Categories
Auth
Dependency manager
Swift PM
Swift version
5
CLI version
12.4.0
Xcode version
15.0.1
Relevant log output
Is this a regression?
Yes
Regression additional context
No response
Platforms
iOS
OS Version
iOS 17.1.1
Device
iPhone 17 - Simulator
Specific to simulators
No response
Additional context
At every single request that we make to our BE API we do run the following code to retrieve the Cognito id token:
Flow:
In some instances, following an unexpected error, the session gets removed from the keychain, and attempting to fetch the auth session results in a 'signedOut' state error.
The text was updated successfully, but these errors were encountered: