Skip to content

Commit

Permalink
fix(Auth): Throw error if hosted UI is not presented during sign out (#…
Browse files Browse the repository at this point in the history 
…3769)

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

* add unit test for hostedUI error

* fix swift lint

* Update AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift

Co-authored-by: Sebastian Villena <[email protected]>

* Update AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift

Co-authored-by: Sebastian Villena <[email protected]>

* worked on review comments

---------

Co-authored-by: Sebastian Villena <[email protected]>
  • Loading branch information
@harsh62 @ruisebas
harsh62 and ruisebas authored Jul 8, 2024
1 parent b243384 commit 5684f34
Show file tree
Hide file tree
Showing 15 changed files with 116 additions and 68 deletions.
52 changes: 22 additions & 30 deletions AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift
View file
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
4 changes: 4 additions & 0 deletions ...Plugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Errors/HostedUIError.swift
View file
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
18 changes: 15 additions & 3 deletions ...yPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Errors/SignOutError.swift
View file
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
}
}
}
8 changes: 4 additions & 4 deletions ...yPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Events/SignOutEvent.swift
View file
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
13 changes: 11 additions & 2 deletions ...gins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/ErrorMapping/SignInError+Helper.swift
View file
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
4 changes: 2 additions & 2 deletions ...h/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignOut/SignOutState+Resolver.swift
View file
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
4 changes: 4 additions & 0 deletions ...lugins/Auth/Sources/AWSCognitoAuthPlugin/Support/Constants/AuthPluginErrorConstants.swift
View file
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
36 changes: 13 additions & 23 deletions ...th/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/SignOut/ShowHostedUISignOutTests.swift
View file
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
11 changes: 11 additions & 0 deletions ...fyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Mocks/MockData/SignedInData+Mock.swift
View file
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)
}
}
6 changes: 6 additions & 0 deletions ...WSCognitoAuthPluginUnitTests/ResolverTests/AuthorizationState/AuthorizationTestData.swift
View file
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
2 changes: 1 addition & 1 deletion ...WSCognitoAuthPluginUnitTests/ResolverTests/SignOutState/SignOutStateNotStartedTests.swift
View file
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

0 comments on commit 5684f34

Please sign in to comment.