Skip to content

Commit dd54ff6

Browse files
Merge pull request #3678 from RedisInsight/feature/RI-5933-enhance-sso-errors
enhance sso errors
2 parents 323c3ff + b7244a5 commit dd54ff6

File tree

8 files changed

+83
-27
lines changed

8 files changed

+83
-27
lines changed

redisinsight/api/src/constants/custom-error-codes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export enum CustomErrorCodes {
1313
CloudOauthUnknownAuthorizationRequest = 11_007,
1414
CloudOauthUnexpectedError = 11_008,
1515
CloudOauthMissedRequiredData = 11_009,
16+
CloudOauthCanceled = 11_010,
1617
CloudCapiUnauthorized = 11_021,
1718
CloudCapiKeyUnauthorized = 11_022,
1819
CloudCapiKeyNotFound = 11_023,

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,11 @@ export default {
8383

8484
CLOUD_CAPI_KEY_UNAUTHORIZED: 'Unable to authorize such CAPI key',
8585

86+
CLOUD_OAUTH_CANCELED: 'Authorization request was canceled.',
8687
CLOUD_OAUTH_MISCONFIGURATION: 'Authorization server misconfiguration.',
8788
CLOUD_OAUTH_GITHUB_EMAIL_PERMISSION: 'Unable to get an email from the GitHub account. Make sure that it is available.',
8889
CLOUD_OAUTH_MISSED_REQUIRED_DATA: 'Unable to get required data from the user profile.',
89-
CLOUD_OAUTH_GITHUB_UNKNOWN_AUTHORIZATION_REQUEST: 'Unknown authorization request.',
90+
CLOUD_OAUTH_UNKNOWN_AUTHORIZATION_REQUEST: 'Unknown authorization request.',
9091
CLOUD_OAUTH_UNEXPECTED_ERROR: 'Unexpected error.',
9192

9293
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: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ 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+
CloudOauthCanceledException,
12+
CloudOauthGithubEmailPermissionException,
1113
CloudOauthMisconfigurationException, CloudOauthMissedRequiredDataException,
14+
CloudOauthUnexpectedErrorException,
1215
CloudOauthUnknownAuthorizationRequestException,
1316
} from 'src/modules/cloud/auth/exceptions';
1417
import { CloudAuthRequestInfo, CloudAuthResponse, CloudAuthStatus } from 'src/modules/cloud/auth/models';
@@ -32,14 +35,30 @@ export class CloudAuthService {
3235
private readonly eventEmitter: EventEmitter2,
3336
) {}
3437

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-
});
38+
static getAuthorizationServerRedirectError(
39+
query: { error_description: string, error: string },
40+
authRequest?: CloudAuthRequest,
41+
) {
42+
if (query?.error_description?.indexOf('canceled') > -1) {
43+
return new CloudOauthCanceledException();
44+
}
45+
46+
if (
47+
query?.error_description?.indexOf('propert') > -1
48+
&& query?.error_description?.indexOf('required') > -1
49+
&& query?.error_description?.indexOf('miss') > -1
50+
) {
51+
return (
52+
authRequest?.idpType === CloudAuthIdpType.GitHub
53+
&& query?.error_description?.indexOf('email') > -1
54+
)
55+
? new CloudOauthGithubEmailPermissionException(query.error_description)
56+
: new CloudOauthMissedRequiredDataException(query.error_description, {
57+
description: query.error_description,
58+
});
4059
}
4160

42-
return new CloudOauthMisconfigurationException(undefined, {
61+
return new CloudOauthUnexpectedErrorException(undefined, {
4362
description: query.error_description,
4463
});
4564
}
@@ -68,17 +87,21 @@ export class CloudAuthService {
6887
callback?: Function,
6988
},
7089
): Promise<string> {
71-
const authRequest: any = await this.getAuthStrategy(options?.strategy).generateAuthRequest(sessionMetadata);
72-
authRequest.callback = options?.callback;
73-
authRequest.action = options?.action;
90+
try {
91+
const authRequest: any = await this.getAuthStrategy(options?.strategy).generateAuthRequest(sessionMetadata);
92+
authRequest.callback = options?.callback;
93+
authRequest.action = options?.action;
7494

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);
95+
// based on requirements we must support only single auth request at the moment
96+
// and logout user before
97+
await this.logout(sessionMetadata);
98+
this.authRequests.clear();
99+
this.authRequests.set(authRequest.state, authRequest);
80100

81-
return CloudAuthStrategy.generateAuthUrl(authRequest).toString();
101+
return CloudAuthStrategy.generateAuthUrl(authRequest).toString();
102+
} catch (e) {
103+
throw new CloudOauthMisconfigurationException();
104+
}
82105
}
83106

84107
/**
@@ -137,12 +160,12 @@ export class CloudAuthService {
137160
throw new CloudOauthUnknownAuthorizationRequestException();
138161
}
139162

163+
const authRequest = this.authRequests.get(query.state);
164+
140165
if (query?.error) {
141-
throw CloudAuthService.getAuthorizationServerRedirectError(query);
166+
throw CloudAuthService.getAuthorizationServerRedirectError(query, authRequest);
142167
}
143168

144-
const authRequest = this.authRequests.get(query.state);
145-
146169
// delete authRequest on this step
147170
// allow to redirect with authorization code only once
148171
this.authRequests.delete(query.state);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import { HttpException, HttpExceptionOptions } from '@nestjs/common';
2+
import { CustomErrorCodes } from 'src/constants';
3+
import ERROR_MESSAGES from 'src/constants/error-messages';
4+
5+
export class CloudOauthCanceledException extends HttpException {
6+
constructor(message = ERROR_MESSAGES.CLOUD_OAUTH_CANCELED, options?: HttpExceptionOptions) {
7+
const response = {
8+
message,
9+
statusCode: 499,
10+
error: 'CloudOauthCanceled',
11+
errorCode: CustomErrorCodes.CloudOauthCanceled,
12+
};
13+
14+
super(response, response.statusCode, options);
15+
}
16+
}

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/api/src/modules/cloud/auth/exceptions/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
export * from './cloud-oauth.canceled.exception';
12
export * from './cloud-oauth.misconfiguration.exception';
23
export * from './cloud-oauth.github-email-permission.exception';
34
export * from './cloud-oauth.missed-required-data.exception';

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,21 @@ 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+
} else if (error instanceof Error) {
19+
errorResponse = new CloudOauthUnexpectedErrorException(error.message).getResponse()
20+
}
21+
22+
return {
23+
status: CloudAuthStatus.Failed,
24+
error: errorResponse,
25+
}
26+
}
27+
1328
export const getTokenCallbackFunction = (webContents: WebContents) => (response: CloudAuthResponse) => {
1429
webContents.send(IpcOnEvent.cloudOauthCallback, response)
1530
webContents.focus()
@@ -42,10 +57,9 @@ export const initCloudOauthHandlers = () => {
4257
} catch (e) {
4358
log.error(wrapErrorMessageSensitiveData(e as Error))
4459

45-
return {
46-
status: CloudAuthStatus.Failed,
47-
error: (new CloudOauthUnexpectedErrorException()).getResponse(),
48-
}
60+
const [currentWindow] = getWindows().values()
61+
62+
currentWindow?.webContents.send(IpcOnEvent.cloudOauthCallback, getOauthIpcErrorResponse(e))
4963
}
5064
})
5165
}

0 commit comments

Comments
 (0)