Skip to content

Commit

Permalink
fix(auth): auth should not throw errors, instead should return them
Browse files Browse the repository at this point in the history
  • Loading branch information
seiyria committed Sep 21, 2023
1 parent ff25e16 commit 7444cdd
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 31 deletions.
1 change: 0 additions & 1 deletion client/src/app/pages/crafting/crafting.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ export class CraftingPage implements OnInit {
updateDiscoveries() {
this.userService.getDiscoveries().subscribe(({ discoveries }: any) => {
this.discoveries = discoveries;
console.log(discoveries);
});
}

Expand Down
2 changes: 1 addition & 1 deletion client/src/app/pages/login/login.page.html
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
</ion-col>
</ion-row>

<ion-row *ngIf="announcement">
<ion-row *ngIf="announcement && announcement.title">
<ion-col class="blog-container">
<ion-card>
<ion-card-header>
Expand Down
21 changes: 18 additions & 3 deletions client/src/app/pages/login/login.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,12 @@ export class LoginPage implements OnInit {
this.authService
.login(this.loginForm.value.email, this.loginForm.value.password)
.subscribe({
next: () => {
next: (res: any) => {
if (res.error) {
this.loginError = res.error;
return;
}

this.router.navigate(['/']);
this.notificationService.getNotifications();
},
Expand All @@ -150,7 +155,12 @@ export class LoginPage implements OnInit {
this.registerForm.value.username,
)
.subscribe({
next: () => {
next: (res: any) => {
if (res.error) {
this.registerError = res.error;
return;
}

if (
!this.registerForm.value.email ||
!this.registerForm.value.password
Expand All @@ -163,7 +173,12 @@ export class LoginPage implements OnInit {
this.registerForm.value.password,
)
.subscribe({
next: () => {
next: (res: any) => {
if (res.error) {
this.registerError = res.error;
return;
}

this.router.navigate(['/']);
this.notificationService.getNotifications();
},
Expand Down
10 changes: 3 additions & 7 deletions server/src/modules/auth/auth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,13 @@ export class AuthController {
@Post('register')
signUp(@Body() signInDto: Record<string, any>) {
if (!signInDto.username || !signInDto.password || !signInDto.email)
throw new BadRequestException('Missing username, password or email');
return { error: 'Missing username, password or email' };

if (signInDto.username.length < 2 || signInDto.username.length > 20)
throw new BadRequestException(
'Username must be between 2 and 20 characters long',
);
return { error: 'Username must be between 2 and 20 characters long' };

if (signInDto.password.length < 8)
throw new BadRequestException(
'Password must be at least 8 characters long',
);
return { error: 'Password must be at least 8 characters long' };

return this.authService.signUp(
signInDto.username,
Expand Down
30 changes: 12 additions & 18 deletions server/src/modules/auth/auth.service.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
import {
BadRequestException,
Injectable,
UnauthorizedException,
} from '@nestjs/common';
import { Injectable } from '@nestjs/common';
import { JwtService } from '@nestjs/jwt';
import * as bcrypt from 'bcrypt';
import { sample } from 'lodash';
Expand All @@ -28,11 +24,9 @@ export class AuthService {
username: string,
password: string,
email: string,
): Promise<IFullUser> {
): Promise<IFullUser | { error: string }> {
if (this.contentService.censor.isProfaneIsh(username))
throw new BadRequestException(
'Username is somewhat profane. Please choose again.',
);
return { error: 'Username is somewhat profane. Please choose again.' };

const usersWithUsername = await this.userService.getAllUsersWithUsername(
username,
Expand All @@ -42,7 +36,7 @@ export class AuthService {
);

if (usersWithUsername.length > 9998) {
throw new BadRequestException('Username is not available.');
return { error: `Username ${username} is not available.` };
} else if (usersWithUsername.length >= 1) {
const usedDiscriminators = new Set(
usersWithUsername.map((user) => user.discriminator),
Expand All @@ -53,7 +47,7 @@ export class AuthService {
}

const discriminator = sample(availableDiscriminators);
if (!discriminator) throw new BadRequestException('Failed to create user.');
if (!discriminator) return { error: 'Failed to create user.' };

const hash = await bcrypt.hash(password, 10);
const newUser = new User(username, discriminator, hash, email);
Expand All @@ -62,12 +56,12 @@ export class AuthService {
try {
createdUser = await this.userService.createUser(newUser);
} catch (err) {
throw new BadRequestException(
'Failed to create user. This email might already be in use.',
);
return {
error: 'Failed to create user. This email might already be in use.',
};
}

if (!createdUser) throw new BadRequestException('Failed to create user.');
if (!createdUser) return { error: 'Failed to create user.' };

this.logger.verbose(
`Registered new user: ${createdUser.username}#${createdUser.discriminator}`,
Expand All @@ -81,15 +75,15 @@ export class AuthService {
async signIn(
username: string,
password: string,
): Promise<IFullUser | IHasAccessToken> {
): Promise<IFullUser | IHasAccessToken | { error: string }> {
const user = await this.userService.findUserByEmail(username);
if (!user) {
throw new UnauthorizedException();
return { error: 'Unable to sign in.' };
}

const isMatch = await bcrypt.compare(password, user.password);
if (!isMatch) {
throw new UnauthorizedException();
return { error: 'Unable to sign in.' };
}

await this.userService.updateUserOnlineTimeById(user._id.toString());
Expand Down
3 changes: 2 additions & 1 deletion server/src/utils/http-exception.filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as Rollbar from 'rollbar';

@Catch(HttpException)
export class HttpExceptionFilter implements ExceptionFilter {
private readonly ignoredCodes = [401];
private rollbar: Rollbar;

constructor(@Inject(ROLLBAR_CONFIG) private rollbarConfig) {
Expand All @@ -23,7 +24,7 @@ export class HttpExceptionFilter implements ExceptionFilter {
const request = ctx.getRequest<Request>();
const status = exception.getStatus();

if (this.rollbar) {
if (this.rollbar && !this.ignoredCodes.includes(status)) {
this.rollbar.error(exception);
}

Expand Down

0 comments on commit 7444cdd

Please sign in to comment.