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

fix(Auth): Throw error if hosted UI is not presented during sign out #3769

Merged
merged 6 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,14 @@ class ShowHostedUISignOut: NSObject, Action {

guard let environment = environment as? AuthEnvironment,
let hostedUIEnvironment = environment.hostedUIEnvironment else {
let message = AuthPluginErrorConstants.configurationError
let error = AuthenticationError.configuration(message: message)
let error = HostedUIError.pluginConfiguration(AuthPluginErrorConstants.configurationError)
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
return
}
let hostedUIConfig = hostedUIEnvironment.configuration

guard let callbackURL = URL(string: hostedUIConfig.oauth.signOutRedirectURI),
let callbackURLScheme = callbackURL.scheme else {
let error = AuthenticationError.configuration(message: "Callback URL could not be retrieved")
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
await sendEvent(with: HostedUIError.signOutRedirectURI, dispatcher: dispatcher, environment: environment)
return
}

Expand All @@ -48,13 +45,7 @@ class ShowHostedUISignOut: NSObject, Action {
callbackScheme: callbackURLScheme,
inPrivate: false,
presentationAnchor: signOutEvent.presentationAnchor)

await sendEvent(with: nil, dispatcher: dispatcher, environment: environment)

} catch HostedUIError.signOutURI {
let error = AuthenticationError.configuration(message: "Could not create logout URL")
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
return
} catch {
self.logVerbose("\(#fileID) Received error \(error)", environment: environment)
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
Expand All @@ -65,32 +56,33 @@ class ShowHostedUISignOut: NSObject, Action {
dispatcher: EventDispatcher,
environment: Environment) async {

var hostedUIError: AWSCognitoHostedUIError?
if let hostedUIInternalError = error as? HostedUIError,
case .cancelled = hostedUIInternalError {
let event = SignOutEvent(eventType: .userCancelled)
self.logVerbose("\(#fileID) Sending event \(event.type)", environment: environment)
await dispatcher.send(event)
return
}

if let error = error as? AuthErrorConvertible {
hostedUIError = AWSCognitoHostedUIError(error: error.authError)
let event: SignOutEvent
if let hostedUIInternalError = error as? HostedUIError {
event = SignOutEvent(eventType: .hostedUISignOutError(hostedUIInternalError))
} else if let error = error as? AuthErrorConvertible {
event = getEvent(for: AWSCognitoHostedUIError(error: error.authError))
} else if let error = error {
let serviceError = AuthError.service("HostedUI failed with error",
"", error)
hostedUIError = AWSCognitoHostedUIError(error: serviceError)
let serviceError = AuthError.service(
"HostedUI failed with error",
"",
error
)
event = getEvent(for: AWSCognitoHostedUIError(error: serviceError))
} else {
event = getEvent(for: nil)
}
let event: SignOutEvent
self.logVerbose("\(#fileID) Sending event \(event.type)", environment: environment)
await dispatcher.send(event)
}

private func getEvent(for hostedUIError: AWSCognitoHostedUIError?) -> SignOutEvent {
if self.signOutEvent.globalSignOut {
event = SignOutEvent(eventType: .signOutGlobally(self.signInData,
return SignOutEvent(eventType: .signOutGlobally(self.signInData,
hostedUIError: hostedUIError))
} else {
event = SignOutEvent(eventType: .revokeToken(self.signInData,
return SignOutEvent(eventType: .revokeToken(self.signInData,
hostedUIError: hostedUIError))
}
self.logVerbose("\(#fileID) Sending event \(event.type)", environment: environment)
await dispatcher.send(event)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ enum HostedUIError: Error {

case signOutURI

case signOutRedirectURI

case proofCalculation

case codeValidation
Expand All @@ -23,6 +25,8 @@ enum HostedUIError: Error {

case serviceMessage(String)

case pluginConfiguration(String)

case cancelled

case invalidContext
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,30 @@ import Amplify

enum SignOutError: Error {

case userCancelled
case hostedUI(HostedUIError)

case localSignOut
}

extension SignOutError: AuthErrorConvertible {
var authError: AuthError {
switch self {
case .userCancelled:
return AuthError.service("", "", AWSCognitoAuthError.userCancelled)
case .hostedUI(let error):
return error.authError
case .localSignOut:
return AuthError.unknown("", nil)
}
}
}

extension SignOutError: Equatable {
static func == (lhs: SignOutError, rhs: SignOutError) -> Bool {
switch (lhs, rhs) {
case (.hostedUI, .hostedUI),
(.localSignOut, .localSignOut):
return true
default:
return false
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ struct SignOutEvent: StateMachineEvent {

case signedOutFailure(AuthenticationError)

case userCancelled
case hostedUISignOutError(HostedUIError)
}

let id: String
Expand All @@ -66,8 +66,8 @@ struct SignOutEvent: StateMachineEvent {
return "SignOutEvent.globalSignOutError"
case .signOutGuest:
return "SignOutEvent.signOutGuest"
case .userCancelled:
return "SignOutEvent.userCancelled"
case .hostedUISignOutError:
return "SignOutEvent.hostedUISignOutError"
}
}

Expand All @@ -94,7 +94,7 @@ extension SignOutEvent.EventType: Equatable {
(.signedOutFailure, .signedOutFailure),
(.globalSignOutError, .globalSignOutError),
(.signOutGuest, .signOutGuest),
(.userCancelled, .userCancelled):
(.hostedUISignOutError, .hostedUISignOutError):
return true
default:
return false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,15 @@ extension HostedUIError: AuthErrorConvertible {
AuthPluginErrorConstants.hostedUITokenURI.recoverySuggestion)

case .signOutURI:
return .service(
return .configuration(
AuthPluginErrorConstants.hostedUISignOutURI.errorDescription,
AuthPluginErrorConstants.hostedUISignOutURI.recoverySuggestion)

case .signOutRedirectURI:
return .configuration(
AuthPluginErrorConstants.hostedUISignOutRedirectURI.errorDescription,
AuthPluginErrorConstants.hostedUISignOutRedirectURI.recoverySuggestion)

case .proofCalculation:
return .invalidState(
AuthPluginErrorConstants.hostedUIProofCalculation.errorDescription,
Expand Down Expand Up @@ -107,11 +112,15 @@ extension HostedUIError: AuthErrorConvertible {
case .unableToStartASWebAuthenticationSession:
return .service(
AuthPluginErrorConstants.hostedUIUnableToStartASWebAuthenticationSession.errorDescription,
AuthPluginErrorConstants.hostedUIUnableToStartASWebAuthenticationSession.recoverySuggestion)
AuthPluginErrorConstants.hostedUIUnableToStartASWebAuthenticationSession.recoverySuggestion,
AWSCognitoAuthError.errorLoadingUI)

case .serviceMessage(let message):
return .service(message, AuthPluginErrorConstants.serviceError)

case .pluginConfiguration(let message):
return .configuration(message, AuthPluginErrorConstants.configurationError)

case .unknown:
return .unknown("WebUI signIn encountered an unknown error", nil)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ extension SignOutState {
newState: SignOutState.revokingToken,
actions: [action]
)
case .userCancelled:
case .hostedUISignOutError(let error):
let action = CancelSignOut(signedInData: signedInData)
return .init(newState: .error(.userCancelled), actions: [action])
return .init(newState: .error(.hostedUI(error)), actions: [action])
default:
return .from(oldState)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ enum AuthPluginErrorConstants {
"SignOut URI could not be created",
"Check the configuration to make sure that HostedUI related information are present")

static let hostedUISignOutRedirectURI: AuthPluginErrorString = (
"Callback URL could not be retrieved",
"Check the configuration to make sure that HostedUI related information are present")

static let hostedUIProofCalculation: AuthPluginErrorString = (
"Proof calculation failed",
"Reach out with amplify team via github to raise an issue")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class ShowHostedUISignOutTests: XCTestCase {
return
}

XCTAssertEqual(event.eventType, .userCancelled)
XCTAssertEqual(event.eventType, .hostedUISignOutError(.cancelled))
expectation.fulfill()
},
environment: Defaults.makeDefaultAuthEnvironment(
Expand Down Expand Up @@ -142,21 +142,19 @@ class ShowHostedUISignOutTests: XCTestCase {
await action.execute(
withDispatcher: MockDispatcher { event in
guard let event = event as? SignOutEvent,
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
expectation.fulfill()
return
}

guard let hostedUIError = hostedUIError,
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
XCTFail("Expected AuthError.configuration")
expectation.fulfill()
return
}

XCTAssertEqual(errorDescription, "Could not create logout URL")
XCTAssertEqual(data, signInData)
XCTAssertEqual(errorDescription, "SignOut URI could not be created")
XCTAssertNil(serviceError)
expectation.fulfill()
},
Expand Down Expand Up @@ -185,22 +183,20 @@ class ShowHostedUISignOutTests: XCTestCase {
await action.execute(
withDispatcher: MockDispatcher { event in
guard let event = event as? SignOutEvent,
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
XCTFail("Expected SignOutEvent.hostedUISignOutError, got \(event)")
expectation.fulfill()
return
}

guard let hostedUIError = hostedUIError,
case .invalidState(let errorDescription, let recoverySuggestion, let serviceError) = hostedUIError.error else {
guard case .invalidState(let errorDescription, let recoverySuggestion, let serviceError) = hostedUIError.authError else {
XCTFail("Expected AuthError.invalidState")
expectation.fulfill()
return
}

XCTAssertEqual(errorDescription, AuthPluginErrorConstants.hostedUIInvalidPresentation.errorDescription)
XCTAssertEqual(recoverySuggestion, AuthPluginErrorConstants.hostedUIInvalidPresentation.recoverySuggestion)
XCTAssertEqual(data, signInData)
XCTAssertNil(serviceError)
expectation.fulfill()
},
Expand Down Expand Up @@ -229,21 +225,19 @@ class ShowHostedUISignOutTests: XCTestCase {
await action.execute(
withDispatcher: MockDispatcher { event in
guard let event = event as? SignOutEvent,
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
expectation.fulfill()
return
}

guard let hostedUIError = hostedUIError,
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
XCTFail("Expected AuthError.configuration")
expectation.fulfill()
return
}

XCTAssertEqual(errorDescription, "Callback URL could not be retrieved")
XCTAssertEqual(data, signInData)
XCTAssertNil(serviceError)
expectation.fulfill()
},
Expand All @@ -269,20 +263,18 @@ class ShowHostedUISignOutTests: XCTestCase {
await action.execute(
withDispatcher: MockDispatcher { event in
guard let event = event as? SignOutEvent,
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
expectation.fulfill()
return
}

guard let hostedUIError = hostedUIError,
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
XCTFail("Expected AuthError.configuration")
expectation.fulfill()
return
}

XCTAssertEqual(data, signInData)
XCTAssertEqual(errorDescription, AuthPluginErrorConstants.configurationError)
XCTAssertNil(serviceError)
expectation.fulfill()
Expand All @@ -309,20 +301,18 @@ class ShowHostedUISignOutTests: XCTestCase {
await action.execute(
withDispatcher: MockDispatcher { event in
guard let event = event as? SignOutEvent,
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
expectation.fulfill()
return
}

guard let hostedUIError = hostedUIError,
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
XCTFail("Expected AuthError.configuration")
expectation.fulfill()
return
}

XCTAssertEqual(data, signInData)
XCTAssertEqual(errorDescription, AuthPluginErrorConstants.configurationError)
XCTAssertNil(serviceError)
expectation.fulfill()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,15 @@ extension SignedInData {
signInMethod: .apiBased(.userSRP),
cognitoUserPoolTokens: tokens)
}

static var hostedUISignInData: SignedInData {
let tokens = AWSCognitoUserPoolTokens.testData
return SignedInData(signedInDate: Date(),
signInMethod: .hostedUI(.init(
scopes: [],
providerInfo: .init(authProvider: .google, idpIdentifier: ""),
presentationAnchor: nil,
preferPrivateSession: false)),
cognitoUserPoolTokens: tokens)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ extension AmplifyCredentials {
credentials: .testData)
}

static var hostedUITestData: AmplifyCredentials {
AmplifyCredentials.userPoolAndIdentityPool(signedInData: .hostedUISignInData,
identityID: "identityId",
credentials: AuthAWSCognitoCredentials.testData)
}

static var testDataWithExpiredTokens: AmplifyCredentials {
AmplifyCredentials.userPoolAndIdentityPool(signedInData: .expiredTestData,
identityID: "identityId",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class SignOutStateNotStartedTests: XCTestCase {
case .signOutLocally,
.signedOutSuccess,
.signedOutFailure,
.userCancelled,
.hostedUISignOutError,
.globalSignOutError:
XCTAssertEqual(
resolver.resolve(
Expand Down
Loading
Loading