Skip to content

Commit 5684f34

Browse files
harsh62sebaland
harsh62
and
sebaland
authored
fix(Auth): Throw error if hosted UI is not presented during sign out (#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 <97059974+ruisebas@users.noreply.github.com> * Update AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift Co-authored-by: Sebastian Villena <97059974+ruisebas@users.noreply.github.com> * worked on review comments --------- Co-authored-by: Sebastian Villena <97059974+ruisebas@users.noreply.github.com>
1 parent b243384 commit 5684f34

File tree

15 files changed

+116
-68
lines changed

15 files changed

+116
-68
lines changed

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Actions/SignOut/ShowHostedUISignOut.swift

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,14 @@ class ShowHostedUISignOut: NSObject, Action {
2626

2727
guard let environment = environment as? AuthEnvironment,
2828
let hostedUIEnvironment = environment.hostedUIEnvironment else {
29-
let message = AuthPluginErrorConstants.configurationError
30-
let error = AuthenticationError.configuration(message: message)
29+
let error = HostedUIError.pluginConfiguration(AuthPluginErrorConstants.configurationError)
3130
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
3231
return
3332
}
3433
let hostedUIConfig = hostedUIEnvironment.configuration
35-
3634
guard let callbackURL = URL(string: hostedUIConfig.oauth.signOutRedirectURI),
3735
let callbackURLScheme = callbackURL.scheme else {
38-
let error = AuthenticationError.configuration(message: "Callback URL could not be retrieved")
39-
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
36+
await sendEvent(with: HostedUIError.signOutRedirectURI, dispatcher: dispatcher, environment: environment)
4037
return
4138
}
4239

@@ -48,13 +45,7 @@ class ShowHostedUISignOut: NSObject, Action {
4845
callbackScheme: callbackURLScheme,
4946
inPrivate: false,
5047
presentationAnchor: signOutEvent.presentationAnchor)
51-
5248
await sendEvent(with: nil, dispatcher: dispatcher, environment: environment)
53-
54-
} catch HostedUIError.signOutURI {
55-
let error = AuthenticationError.configuration(message: "Could not create logout URL")
56-
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
57-
return
5849
} catch {
5950
self.logVerbose("\(#fileID) Received error \(error)", environment: environment)
6051
await sendEvent(with: error, dispatcher: dispatcher, environment: environment)
@@ -65,32 +56,33 @@ class ShowHostedUISignOut: NSObject, Action {
6556
dispatcher: EventDispatcher,
6657
environment: Environment) async {
6758

68-
var hostedUIError: AWSCognitoHostedUIError?
69-
if let hostedUIInternalError = error as? HostedUIError,
70-
case .cancelled = hostedUIInternalError {
71-
let event = SignOutEvent(eventType: .userCancelled)
72-
self.logVerbose("\(#fileID) Sending event \(event.type)", environment: environment)
73-
await dispatcher.send(event)
74-
return
75-
}
76-
77-
if let error = error as? AuthErrorConvertible {
78-
hostedUIError = AWSCognitoHostedUIError(error: error.authError)
59+
let event: SignOutEvent
60+
if let hostedUIInternalError = error as? HostedUIError {
61+
event = SignOutEvent(eventType: .hostedUISignOutError(hostedUIInternalError))
62+
} else if let error = error as? AuthErrorConvertible {
63+
event = getEvent(for: AWSCognitoHostedUIError(error: error.authError))
7964
} else if let error = error {
80-
let serviceError = AuthError.service("HostedUI failed with error",
81-
"", error)
82-
hostedUIError = AWSCognitoHostedUIError(error: serviceError)
65+
let serviceError = AuthError.service(
66+
"HostedUI failed with error",
67+
"",
68+
error
69+
)
70+
event = getEvent(for: AWSCognitoHostedUIError(error: serviceError))
71+
} else {
72+
event = getEvent(for: nil)
8373
}
84-
let event: SignOutEvent
74+
self.logVerbose("\(#fileID) Sending event \(event.type)", environment: environment)
75+
await dispatcher.send(event)
76+
}
77+
78+
private func getEvent(for hostedUIError: AWSCognitoHostedUIError?) -> SignOutEvent {
8579
if self.signOutEvent.globalSignOut {
86-
event = SignOutEvent(eventType: .signOutGlobally(self.signInData,
80+
return SignOutEvent(eventType: .signOutGlobally(self.signInData,
8781
hostedUIError: hostedUIError))
8882
} else {
89-
event = SignOutEvent(eventType: .revokeToken(self.signInData,
83+
return SignOutEvent(eventType: .revokeToken(self.signInData,
9084
hostedUIError: hostedUIError))
9185
}
92-
self.logVerbose("\(#fileID) Sending event \(event.type)", environment: environment)
93-
await dispatcher.send(event)
9486
}
9587
}
9688

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Errors/HostedUIError.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ enum HostedUIError: Error {
1515

1616
case signOutURI
1717

18+
case signOutRedirectURI
19+
1820
case proofCalculation
1921

2022
case codeValidation
@@ -23,6 +25,8 @@ enum HostedUIError: Error {
2325

2426
case serviceMessage(String)
2527

28+
case pluginConfiguration(String)
29+
2630
case cancelled
2731

2832
case invalidContext

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Errors/SignOutError.swift

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,30 @@ import Amplify
1010

1111
enum SignOutError: Error {
1212

13-
case userCancelled
13+
case hostedUI(HostedUIError)
1414

1515
case localSignOut
1616
}
1717

1818
extension SignOutError: AuthErrorConvertible {
1919
var authError: AuthError {
2020
switch self {
21-
case .userCancelled:
22-
return AuthError.service("", "", AWSCognitoAuthError.userCancelled)
21+
case .hostedUI(let error):
22+
return error.authError
2323
case .localSignOut:
2424
return AuthError.unknown("", nil)
2525
}
2626
}
2727
}
28+
29+
extension SignOutError: Equatable {
30+
static func == (lhs: SignOutError, rhs: SignOutError) -> Bool {
31+
switch (lhs, rhs) {
32+
case (.hostedUI, .hostedUI),
33+
(.localSignOut, .localSignOut):
34+
return true
35+
default:
36+
return false
37+
}
38+
}
39+
}

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/CodeGen/Events/SignOutEvent.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ struct SignOutEvent: StateMachineEvent {
4141

4242
case signedOutFailure(AuthenticationError)
4343

44-
case userCancelled
44+
case hostedUISignOutError(HostedUIError)
4545
}
4646

4747
let id: String
@@ -66,8 +66,8 @@ struct SignOutEvent: StateMachineEvent {
6666
return "SignOutEvent.globalSignOutError"
6767
case .signOutGuest:
6868
return "SignOutEvent.signOutGuest"
69-
case .userCancelled:
70-
return "SignOutEvent.userCancelled"
69+
case .hostedUISignOutError:
70+
return "SignOutEvent.hostedUISignOutError"
7171
}
7272
}
7373

@@ -94,7 +94,7 @@ extension SignOutEvent.EventType: Equatable {
9494
(.signedOutFailure, .signedOutFailure),
9595
(.globalSignOutError, .globalSignOutError),
9696
(.signOutGuest, .signOutGuest),
97-
(.userCancelled, .userCancelled):
97+
(.hostedUISignOutError, .hostedUISignOutError):
9898
return true
9999
default:
100100
return false

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/ErrorMapping/SignInError+Helper.swift

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,15 @@ extension HostedUIError: AuthErrorConvertible {
7474
AuthPluginErrorConstants.hostedUITokenURI.recoverySuggestion)
7575

7676
case .signOutURI:
77-
return .service(
77+
return .configuration(
7878
AuthPluginErrorConstants.hostedUISignOutURI.errorDescription,
7979
AuthPluginErrorConstants.hostedUISignOutURI.recoverySuggestion)
8080

81+
case .signOutRedirectURI:
82+
return .configuration(
83+
AuthPluginErrorConstants.hostedUISignOutRedirectURI.errorDescription,
84+
AuthPluginErrorConstants.hostedUISignOutRedirectURI.recoverySuggestion)
85+
8186
case .proofCalculation:
8287
return .invalidState(
8388
AuthPluginErrorConstants.hostedUIProofCalculation.errorDescription,
@@ -107,11 +112,15 @@ extension HostedUIError: AuthErrorConvertible {
107112
case .unableToStartASWebAuthenticationSession:
108113
return .service(
109114
AuthPluginErrorConstants.hostedUIUnableToStartASWebAuthenticationSession.errorDescription,
110-
AuthPluginErrorConstants.hostedUIUnableToStartASWebAuthenticationSession.recoverySuggestion)
115+
AuthPluginErrorConstants.hostedUIUnableToStartASWebAuthenticationSession.recoverySuggestion,
116+
AWSCognitoAuthError.errorLoadingUI)
111117

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

121+
case .pluginConfiguration(let message):
122+
return .configuration(message, AuthPluginErrorConstants.configurationError)
123+
115124
case .unknown:
116125
return .unknown("WebUI signIn encountered an unknown error", nil)
117126

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/StateMachine/Resolvers/SignOut/SignOutState+Resolver.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ extension SignOutState {
117117
newState: SignOutState.revokingToken,
118118
actions: [action]
119119
)
120-
case .userCancelled:
120+
case .hostedUISignOutError(let error):
121121
let action = CancelSignOut(signedInData: signedInData)
122-
return .init(newState: .error(.userCancelled), actions: [action])
122+
return .init(newState: .error(.hostedUI(error)), actions: [action])
123123
default:
124124
return .from(oldState)
125125
}

‎AmplifyPlugins/Auth/Sources/AWSCognitoAuthPlugin/Support/Constants/AuthPluginErrorConstants.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ enum AuthPluginErrorConstants {
5656
"SignOut URI could not be created",
5757
"Check the configuration to make sure that HostedUI related information are present")
5858

59+
static let hostedUISignOutRedirectURI: AuthPluginErrorString = (
60+
"Callback URL could not be retrieved",
61+
"Check the configuration to make sure that HostedUI related information are present")
62+
5963
static let hostedUIProofCalculation: AuthPluginErrorString = (
6064
"Proof calculation failed",
6165
"Reach out with amplify team via github to raise an issue")

‎AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ActionTests/SignOut/ShowHostedUISignOutTests.swift

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ class ShowHostedUISignOutTests: XCTestCase {
114114
return
115115
}
116116

117-
XCTAssertEqual(event.eventType, .userCancelled)
117+
XCTAssertEqual(event.eventType, .hostedUISignOutError(.cancelled))
118118
expectation.fulfill()
119119
},
120120
environment: Defaults.makeDefaultAuthEnvironment(
@@ -142,21 +142,19 @@ class ShowHostedUISignOutTests: XCTestCase {
142142
await action.execute(
143143
withDispatcher: MockDispatcher { event in
144144
guard let event = event as? SignOutEvent,
145-
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
145+
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
146146
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
147147
expectation.fulfill()
148148
return
149149
}
150150

151-
guard let hostedUIError = hostedUIError,
152-
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
151+
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
153152
XCTFail("Expected AuthError.configuration")
154153
expectation.fulfill()
155154
return
156155
}
157156

158-
XCTAssertEqual(errorDescription, "Could not create logout URL")
159-
XCTAssertEqual(data, signInData)
157+
XCTAssertEqual(errorDescription, "SignOut URI could not be created")
160158
XCTAssertNil(serviceError)
161159
expectation.fulfill()
162160
},
@@ -185,22 +183,20 @@ class ShowHostedUISignOutTests: XCTestCase {
185183
await action.execute(
186184
withDispatcher: MockDispatcher { event in
187185
guard let event = event as? SignOutEvent,
188-
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
189-
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
186+
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
187+
XCTFail("Expected SignOutEvent.hostedUISignOutError, got \(event)")
190188
expectation.fulfill()
191189
return
192190
}
193191

194-
guard let hostedUIError = hostedUIError,
195-
case .invalidState(let errorDescription, let recoverySuggestion, let serviceError) = hostedUIError.error else {
192+
guard case .invalidState(let errorDescription, let recoverySuggestion, let serviceError) = hostedUIError.authError else {
196193
XCTFail("Expected AuthError.invalidState")
197194
expectation.fulfill()
198195
return
199196
}
200197

201198
XCTAssertEqual(errorDescription, AuthPluginErrorConstants.hostedUIInvalidPresentation.errorDescription)
202199
XCTAssertEqual(recoverySuggestion, AuthPluginErrorConstants.hostedUIInvalidPresentation.recoverySuggestion)
203-
XCTAssertEqual(data, signInData)
204200
XCTAssertNil(serviceError)
205201
expectation.fulfill()
206202
},
@@ -229,21 +225,19 @@ class ShowHostedUISignOutTests: XCTestCase {
229225
await action.execute(
230226
withDispatcher: MockDispatcher { event in
231227
guard let event = event as? SignOutEvent,
232-
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
228+
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
233229
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
234230
expectation.fulfill()
235231
return
236232
}
237233

238-
guard let hostedUIError = hostedUIError,
239-
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
234+
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
240235
XCTFail("Expected AuthError.configuration")
241236
expectation.fulfill()
242237
return
243238
}
244239

245240
XCTAssertEqual(errorDescription, "Callback URL could not be retrieved")
246-
XCTAssertEqual(data, signInData)
247241
XCTAssertNil(serviceError)
248242
expectation.fulfill()
249243
},
@@ -269,20 +263,18 @@ class ShowHostedUISignOutTests: XCTestCase {
269263
await action.execute(
270264
withDispatcher: MockDispatcher { event in
271265
guard let event = event as? SignOutEvent,
272-
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
266+
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
273267
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
274268
expectation.fulfill()
275269
return
276270
}
277271

278-
guard let hostedUIError = hostedUIError,
279-
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
272+
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
280273
XCTFail("Expected AuthError.configuration")
281274
expectation.fulfill()
282275
return
283276
}
284277

285-
XCTAssertEqual(data, signInData)
286278
XCTAssertEqual(errorDescription, AuthPluginErrorConstants.configurationError)
287279
XCTAssertNil(serviceError)
288280
expectation.fulfill()
@@ -309,20 +301,18 @@ class ShowHostedUISignOutTests: XCTestCase {
309301
await action.execute(
310302
withDispatcher: MockDispatcher { event in
311303
guard let event = event as? SignOutEvent,
312-
case .signOutGlobally(let data, let hostedUIError) = event.eventType else {
304+
case .hostedUISignOutError(let hostedUIError) = event.eventType else {
313305
XCTFail("Expected SignOutEvent.signOutGlobally, got \(event)")
314306
expectation.fulfill()
315307
return
316308
}
317309

318-
guard let hostedUIError = hostedUIError,
319-
case .configuration(let errorDescription, _, let serviceError) = hostedUIError.error else {
310+
guard case .configuration(let errorDescription, _, let serviceError) = hostedUIError.authError else {
320311
XCTFail("Expected AuthError.configuration")
321312
expectation.fulfill()
322313
return
323314
}
324315

325-
XCTAssertEqual(data, signInData)
326316
XCTAssertEqual(errorDescription, AuthPluginErrorConstants.configurationError)
327317
XCTAssertNil(serviceError)
328318
expectation.fulfill()

‎AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/Mocks/MockData/SignedInData+Mock.swift

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,15 @@ extension SignedInData {
2323
signInMethod: .apiBased(.userSRP),
2424
cognitoUserPoolTokens: tokens)
2525
}
26+
27+
static var hostedUISignInData: SignedInData {
28+
let tokens = AWSCognitoUserPoolTokens.testData
29+
return SignedInData(signedInDate: Date(),
30+
signInMethod: .hostedUI(.init(
31+
scopes: [],
32+
providerInfo: .init(authProvider: .google, idpIdentifier: ""),
33+
presentationAnchor: nil,
34+
preferPrivateSession: false)),
35+
cognitoUserPoolTokens: tokens)
36+
}
2637
}

‎AmplifyPlugins/Auth/Tests/AWSCognitoAuthPluginUnitTests/ResolverTests/AuthorizationState/AuthorizationTestData.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,12 @@ extension AmplifyCredentials {
5353
credentials: .testData)
5454
}
5555

56+
static var hostedUITestData: AmplifyCredentials {
57+
AmplifyCredentials.userPoolAndIdentityPool(signedInData: .hostedUISignInData,
58+
identityID: "identityId",
59+
credentials: AuthAWSCognitoCredentials.testData)
60+
}
61+
5662
static var testDataWithExpiredTokens: AmplifyCredentials {
5763
AmplifyCredentials.userPoolAndIdentityPool(signedInData: .expiredTestData,
5864
identityID: "identityId",

0 commit comments

Comments
 (0)