Skip to content

Commit 8da7e08

Browse files
enhance sso errors
1 parent 5b02a83 commit 8da7e08

File tree

5 files changed

+59
-26
lines changed

5 files changed

+59
-26
lines changed

redisinsight/api/src/constants/error-messages.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ export default {
8686
CLOUD_OAUTH_MISCONFIGURATION: 'Authorization server misconfiguration.',
8787
CLOUD_OAUTH_GITHUB_EMAIL_PERMISSION: 'Unable to get an email from the GitHub account. Make sure that it is available.',
8888
CLOUD_OAUTH_MISSED_REQUIRED_DATA: 'Unable to get required data from the user profile.',
89-
CLOUD_OAUTH_GITHUB_UNKNOWN_AUTHORIZATION_REQUEST: 'Unknown authorization request.',
89+
CLOUD_OAUTH_UNKNOWN_AUTHORIZATION_REQUEST: 'Unknown authorization request.',
9090
CLOUD_OAUTH_UNEXPECTED_ERROR: 'Unexpected error.',
9191

9292
CLOUD_JOB_UNEXPECTED_ERROR: 'Unexpected error occurred',

redisinsight/api/src/modules/cloud/auth/cloud-auth.service.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -223,10 +223,10 @@ describe('CloudAuthService', () => {
223223
await expect(service['callback'](
224224
{
225225
...mockCloudAuthGoogleCallbackQueryObject,
226-
error: 'bad request',
227-
error_description: 'Some properties are missing: email and lastName',
226+
error: 'access_denied',
227+
error_description: 'Some required properties are missing: email and lastName',
228228
},
229-
)).rejects.toThrow(new CloudOauthMissedRequiredDataException('Some properties are missing: email and lastName'));
229+
)).rejects.toThrow(new CloudOauthMissedRequiredDataException('Some required properties are missing: email and lastName'));
230230
});
231231
it('should throw an error if request not found', async () => {
232232
expect(service['authRequests'].size).toEqual(1);

redisinsight/api/src/modules/cloud/auth/cloud-auth.service.ts

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { CloudSessionService } from 'src/modules/cloud/session/cloud-session.ser
88
import { GithubIdpCloudAuthStrategy } from 'src/modules/cloud/auth/auth-strategy/github-idp.cloud.auth-strategy';
99
import { wrapHttpError } from 'src/common/utils';
1010
import {
11+
CloudOauthGithubEmailPermissionException,
1112
CloudOauthMisconfigurationException, CloudOauthMissedRequiredDataException,
1213
CloudOauthUnknownAuthorizationRequestException,
1314
} from 'src/modules/cloud/auth/exceptions';
@@ -32,11 +33,23 @@ export class CloudAuthService {
3233
private readonly eventEmitter: EventEmitter2,
3334
) {}
3435

35-
static getAuthorizationServerRedirectError(query: { error_description: string }) {
36-
if (query?.error_description?.indexOf('properties are missing') > -1) {
37-
return new CloudOauthMissedRequiredDataException(query.error_description, {
38-
description: query.error_description,
39-
});
36+
static getAuthorizationServerRedirectError(
37+
query: { error_description: string, error: string },
38+
authRequest?: CloudAuthRequest,
39+
) {
40+
if (
41+
query?.error_description?.indexOf('propert') > -1
42+
&& query?.error_description?.indexOf('required') > -1
43+
&& query?.error_description?.indexOf('miss') > -1
44+
) {
45+
return (
46+
authRequest?.idpType === CloudAuthIdpType.GitHub
47+
&& query?.error_description?.indexOf('email') > -1
48+
)
49+
? new CloudOauthGithubEmailPermissionException(query.error_description)
50+
: new CloudOauthMissedRequiredDataException(query.error_description, {
51+
description: query.error_description,
52+
});
4053
}
4154

4255
return new CloudOauthMisconfigurationException(undefined, {
@@ -68,17 +81,21 @@ export class CloudAuthService {
6881
callback?: Function,
6982
},
7083
): Promise<string> {
71-
const authRequest: any = await this.getAuthStrategy(options?.strategy).generateAuthRequest(sessionMetadata);
72-
authRequest.callback = options?.callback;
73-
authRequest.action = options?.action;
84+
try {
85+
const authRequest: any = await this.getAuthStrategy(options?.strategy).generateAuthRequest(sessionMetadata);
86+
authRequest.callback = options?.callback;
87+
authRequest.action = options?.action;
7488

75-
// based on requirements we must support only single auth request at the moment
76-
// and logout user before
77-
await this.logout(sessionMetadata);
78-
this.authRequests.clear();
79-
this.authRequests.set(authRequest.state, authRequest);
89+
// based on requirements we must support only single auth request at the moment
90+
// and logout user before
91+
await this.logout(sessionMetadata);
92+
this.authRequests.clear();
93+
this.authRequests.set(authRequest.state, authRequest);
8094

81-
return CloudAuthStrategy.generateAuthUrl(authRequest).toString();
95+
return CloudAuthStrategy.generateAuthUrl(authRequest).toString();
96+
} catch (e) {
97+
throw new CloudOauthMisconfigurationException();
98+
}
8299
}
83100

84101
/**
@@ -137,12 +154,12 @@ export class CloudAuthService {
137154
throw new CloudOauthUnknownAuthorizationRequestException();
138155
}
139156

157+
const authRequest = this.authRequests.get(query.state);
158+
140159
if (query?.error) {
141-
throw CloudAuthService.getAuthorizationServerRedirectError(query);
160+
throw CloudAuthService.getAuthorizationServerRedirectError(query, authRequest);
142161
}
143162

144-
const authRequest = this.authRequests.get(query.state);
145-
146163
// delete authRequest on this step
147164
// allow to redirect with authorization code only once
148165
this.authRequests.delete(query.state);

redisinsight/api/src/modules/cloud/auth/exceptions/cloud-oauth.unknown-authorization-request.exception.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { CustomErrorCodes } from 'src/constants';
33
import ERROR_MESSAGES from 'src/constants/error-messages';
44

55
export class CloudOauthUnknownAuthorizationRequestException extends HttpException {
6-
constructor(message = ERROR_MESSAGES.CLOUD_OAUTH_GITHUB_UNKNOWN_AUTHORIZATION_REQUEST, options?: HttpExceptionOptions) {
6+
constructor(message = ERROR_MESSAGES.CLOUD_OAUTH_UNKNOWN_AUTHORIZATION_REQUEST, options?: HttpExceptionOptions) {
77
const response = {
88
message,
99
statusCode: HttpStatus.BAD_REQUEST,

redisinsight/desktop/src/lib/cloud/cloud-oauth.handlers.ts

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,23 @@ import { DEFAULT_SESSION_ID, DEFAULT_USER_ID } from 'apiSrc/common/constants'
1010
import { CloudOauthUnexpectedErrorException } from 'apiSrc/modules/cloud/auth/exceptions'
1111
import { CloudAuthService } from '../../../../api/dist/src/modules/cloud/auth/cloud-auth.service'
1212

13+
export const getOauthIpcErrorResponse = (error: any): { status: CloudAuthStatus.Failed, error: {} } => {
14+
let errorResponse = new CloudOauthUnexpectedErrorException().getResponse()
15+
16+
if (error?.getResponse) {
17+
errorResponse = error.getResponse()
18+
}
19+
20+
if (error instanceof Error) {
21+
errorResponse = new CloudOauthUnexpectedErrorException(error.message).getResponse()
22+
}
23+
24+
return {
25+
status: CloudAuthStatus.Failed,
26+
error: errorResponse,
27+
}
28+
}
29+
1330
export const getTokenCallbackFunction = (webContents: WebContents) => (response: CloudAuthResponse) => {
1431
webContents.send(IpcOnEvent.cloudOauthCallback, response)
1532
webContents.focus()
@@ -42,10 +59,9 @@ export const initCloudOauthHandlers = () => {
4259
} catch (e) {
4360
log.error(wrapErrorMessageSensitiveData(e as Error))
4461

45-
return {
46-
status: CloudAuthStatus.Failed,
47-
error: (new CloudOauthUnexpectedErrorException()).getResponse(),
48-
}
62+
const [currentWindow] = getWindows().values()
63+
64+
currentWindow?.webContents.send(IpcOnEvent.cloudOauthCallback, getOauthIpcErrorResponse(e))
4965
}
5066
})
5167
}

0 commit comments

Comments
 (0)