Skip to content

Commit 08e5fa9

Browse files
refactor(backend): enhance auth strategies with type safety and better error handling (hoppscotch#5066)
Co-authored-by: mirarifhasan <[email protected]>
1 parent 19362a4 commit 08e5fa9

File tree

6 files changed

+74
-41
lines changed

6 files changed

+74
-41
lines changed

packages/hoppscotch-backend/src/auth/strategies/github.strategy.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
import { Strategy } from 'passport-github2';
1+
import { Strategy, Profile } from 'passport-github2';
22
import { PassportStrategy } from '@nestjs/passport';
33
import { Injectable, UnauthorizedException } from '@nestjs/common';
44
import { AuthService } from '../auth.service';
55
import { UserService } from 'src/user/user.service';
66
import * as O from 'fp-ts/Option';
77
import * as E from 'fp-ts/Either';
88
import { ConfigService } from '@nestjs/config';
9+
import { validateEmail } from 'src/utils';
10+
import { AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH } from 'src/errors';
911

1012
@Injectable()
1113
export class GithubStrategy extends PassportStrategy(Strategy) {
@@ -15,18 +17,26 @@ export class GithubStrategy extends PassportStrategy(Strategy) {
1517
private configService: ConfigService,
1618
) {
1719
super({
18-
clientID: configService.get('INFRA.GITHUB_CLIENT_ID'),
19-
clientSecret: configService.get('INFRA.GITHUB_CLIENT_SECRET'),
20-
callbackURL: configService.get('INFRA.GITHUB_CALLBACK_URL'),
21-
scope: [configService.get('INFRA.GITHUB_SCOPE')],
20+
clientID: configService.get<string>('INFRA.GITHUB_CLIENT_ID'),
21+
clientSecret: configService.get<string>('INFRA.GITHUB_CLIENT_SECRET'),
22+
callbackURL: configService.get<string>('INFRA.GITHUB_CALLBACK_URL'),
23+
scope: [configService.get<string>('INFRA.GITHUB_SCOPE')],
2224
store: true,
2325
});
2426
}
2527

26-
async validate(accessToken, refreshToken, profile, done) {
27-
const user = await this.usersService.findUserByEmail(
28-
profile.emails[0].value,
29-
);
28+
async validate(
29+
accessToken: string,
30+
refreshToken: string,
31+
profile: Profile,
32+
done,
33+
) {
34+
const email = profile.emails?.[0].value;
35+
36+
if (!validateEmail(email))
37+
throw new UnauthorizedException(AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH);
38+
39+
const user = await this.usersService.findUserByEmail(email);
3040

3141
if (O.isNone(user)) {
3242
const createdUser = await this.usersService.createUserSSO(
@@ -38,7 +48,7 @@ export class GithubStrategy extends PassportStrategy(Strategy) {
3848
}
3949

4050
/**
41-
* * displayName and photoURL maybe null if user logged-in via magic-link before SSO
51+
* displayName and photoURL maybe null if user logged-in via magic-link before SSO
4252
*/
4353
if (!user.value.displayName || !user.value.photoURL) {
4454
const updatedUser = await this.usersService.updateUserDetails(
@@ -51,8 +61,8 @@ export class GithubStrategy extends PassportStrategy(Strategy) {
5161
}
5262

5363
/**
54-
* * Check to see if entry for Github is present in the Account table for user
55-
* * If user was created with another provider findUserByEmail may return true
64+
* Check to see if entry for Github is present in the Account table for user
65+
* If user was created with another provider findUserByEmail may return true
5666
*/
5767
const providerAccountExists =
5868
await this.authService.checkIfProviderAccountExists(user.value, profile);

packages/hoppscotch-backend/src/auth/strategies/google.strategy.ts

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
1-
import { Strategy, VerifyCallback } from 'passport-google-oauth20';
1+
import { Strategy, VerifyCallback, Profile } from 'passport-google-oauth20';
22
import { PassportStrategy } from '@nestjs/passport';
33
import { Injectable, UnauthorizedException } from '@nestjs/common';
44
import { UserService } from 'src/user/user.service';
55
import * as O from 'fp-ts/Option';
66
import { AuthService } from '../auth.service';
77
import * as E from 'fp-ts/Either';
88
import { ConfigService } from '@nestjs/config';
9+
import { Request } from 'express';
10+
import { validateEmail } from 'src/utils';
11+
import { AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH } from 'src/errors';
912

1013
@Injectable()
1114
export class GoogleStrategy extends PassportStrategy(Strategy) {
@@ -15,25 +18,28 @@ export class GoogleStrategy extends PassportStrategy(Strategy) {
1518
private configService: ConfigService,
1619
) {
1720
super({
18-
clientID: configService.get('INFRA.GOOGLE_CLIENT_ID'),
19-
clientSecret: configService.get('INFRA.GOOGLE_CLIENT_SECRET'),
20-
callbackURL: configService.get('INFRA.GOOGLE_CALLBACK_URL'),
21-
scope: configService.get('INFRA.GOOGLE_SCOPE').split(','),
21+
clientID: configService.get<string>('INFRA.GOOGLE_CLIENT_ID'),
22+
clientSecret: configService.get<string>('INFRA.GOOGLE_CLIENT_SECRET'),
23+
callbackURL: configService.get<string>('INFRA.GOOGLE_CALLBACK_URL'),
24+
scope: configService.get<string>('INFRA.GOOGLE_SCOPE').split(','),
2225
passReqToCallback: true,
2326
store: true,
2427
});
2528
}
2629

2730
async validate(
2831
req: Request,
29-
accessToken,
30-
refreshToken,
31-
profile,
32+
accessToken: string,
33+
refreshToken: string,
34+
profile: Profile,
3235
done: VerifyCallback,
3336
) {
34-
const user = await this.usersService.findUserByEmail(
35-
profile.emails[0].value,
36-
);
37+
const email = profile.emails?.[0].value;
38+
39+
if (!validateEmail(email))
40+
throw new UnauthorizedException(AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH);
41+
42+
const user = await this.usersService.findUserByEmail(email);
3743

3844
if (O.isNone(user)) {
3945
const createdUser = await this.usersService.createUserSSO(
@@ -45,7 +51,7 @@ export class GoogleStrategy extends PassportStrategy(Strategy) {
4551
}
4652

4753
/**
48-
* * displayName and photoURL maybe null if user logged-in via magic-link before SSO
54+
* displayName and photoURL maybe null if user logged-in via magic-link before SSO
4955
*/
5056
if (!user.value.displayName || !user.value.photoURL) {
5157
const updatedUser = await this.usersService.updateUserDetails(
@@ -58,8 +64,8 @@ export class GoogleStrategy extends PassportStrategy(Strategy) {
5864
}
5965

6066
/**
61-
* * Check to see if entry for Google is present in the Account table for user
62-
* * If user was created with another provider findUserByEmail may return true
67+
* Check to see if entry for Google is present in the Account table for user
68+
* If user was created with another provider findUserByEmail may return true
6369
*/
6470
const providerAccountExists =
6571
await this.authService.checkIfProviderAccountExists(user.value, profile);

packages/hoppscotch-backend/src/auth/strategies/jwt.strategy.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export class JwtStrategy extends PassportStrategy(Strategy, 'jwt') {
9393
),
9494
),
9595
]),
96-
secretOrKey: configService.get('JWT_SECRET'),
96+
secretOrKey: configService.get<string>('JWT_SECRET'),
9797
});
9898
}
9999

packages/hoppscotch-backend/src/auth/strategies/microsoft.strategy.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { UserService } from 'src/user/user.service';
66
import * as O from 'fp-ts/Option';
77
import * as E from 'fp-ts/Either';
88
import { ConfigService } from '@nestjs/config';
9+
import { validateEmail } from 'src/utils';
10+
import { AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH } from 'src/errors';
911

1012
@Injectable()
1113
export class MicrosoftStrategy extends PassportStrategy(Strategy) {
@@ -15,19 +17,27 @@ export class MicrosoftStrategy extends PassportStrategy(Strategy) {
1517
private configService: ConfigService,
1618
) {
1719
super({
18-
clientID: configService.get('INFRA.MICROSOFT_CLIENT_ID'),
19-
clientSecret: configService.get('INFRA.MICROSOFT_CLIENT_SECRET'),
20-
callbackURL: configService.get('INFRA.MICROSOFT_CALLBACK_URL'),
21-
scope: configService.get('INFRA.MICROSOFT_SCOPE').split(','),
22-
tenant: configService.get('INFRA.MICROSOFT_TENANT'),
20+
clientID: configService.get<string>('INFRA.MICROSOFT_CLIENT_ID'),
21+
clientSecret: configService.get<string>('INFRA.MICROSOFT_CLIENT_SECRET'),
22+
callbackURL: configService.get<string>('INFRA.MICROSOFT_CALLBACK_URL'),
23+
scope: configService.get<string>('INFRA.MICROSOFT_SCOPE').split(','),
24+
tenant: configService.get<string>('INFRA.MICROSOFT_TENANT'),
2325
store: true,
2426
});
2527
}
2628

27-
async validate(accessToken: string, refreshToken: string, profile, done) {
28-
const user = await this.usersService.findUserByEmail(
29-
profile.emails[0].value,
30-
);
29+
async validate(
30+
accessToken: string,
31+
refreshToken: string,
32+
profile,
33+
done,
34+
) {
35+
const email = profile?.emails?.[0]?.value;
36+
37+
if (!validateEmail(email))
38+
throw new UnauthorizedException(AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH);
39+
40+
const user = await this.usersService.findUserByEmail(email);
3141

3242
if (O.isNone(user)) {
3343
const createdUser = await this.usersService.createUserSSO(
@@ -39,7 +49,7 @@ export class MicrosoftStrategy extends PassportStrategy(Strategy) {
3949
}
4050

4151
/**
42-
* * displayName and photoURL maybe null if user logged-in via magic-link before SSO
52+
* displayName and photoURL maybe null if user logged-in via magic-link before SSO
4353
*/
4454
if (!user.value.displayName || !user.value.photoURL) {
4555
const updatedUser = await this.usersService.updateUserDetails(
@@ -52,8 +62,8 @@ export class MicrosoftStrategy extends PassportStrategy(Strategy) {
5262
}
5363

5464
/**
55-
* * Check to see if entry for Microsoft is present in the Account table for user
56-
* * If user was created with another provider findUserByEmail may return true
65+
* Check to see if entry for Microsoft is present in the Account table for user
66+
* If user was created with another provider findUserByEmail may return true
5767
*/
5868
const providerAccountExists =
5969
await this.authService.checkIfProviderAccountExists(user.value, profile);

packages/hoppscotch-backend/src/auth/strategies/rt-jwt.strategy.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,15 @@ export class RTJwtStrategy extends PassportStrategy(Strategy, 'jwt-refresh') {
2525
super({
2626
jwtFromRequest: ExtractJwt.fromExtractors([
2727
(request: Request) => {
28-
const RTCookie = request.cookies['refresh_token'];
28+
const RTCookie = request.cookies?.['refresh_token'];
2929
if (!RTCookie) {
3030
console.error('`refresh_token` not found');
3131
throw new ForbiddenException(COOKIES_NOT_FOUND);
3232
}
3333
return RTCookie;
3434
},
3535
]),
36-
secretOrKey: configService.get('JWT_SECRET'),
36+
secretOrKey: configService.get<string>('JWT_SECRET'),
3737
});
3838
}
3939

packages/hoppscotch-backend/src/errors.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ export const AUTH_PROVIDER_NOT_SPECIFIED = 'auth/provider_not_specified';
4343
export const AUTH_PROVIDER_NOT_CONFIGURED =
4444
'auth/provider_not_configured_correctly';
4545

46+
/**
47+
* Email not provided by OAuth provider
48+
* (SSO Strategies)
49+
*/
50+
export const AUTH_EMAIL_NOT_PROVIDED_BY_OAUTH =
51+
'auth/email_not_provided_by_oauth';
52+
4653
/**
4754
* Environment variable "VITE_ALLOWED_AUTH_PROVIDERS" is not present in .env file
4855
*/

0 commit comments

Comments
 (0)