Skip to content

Commit

Permalink
Fix a bug where a route marked with Public will fail when unauthentic…
Browse files Browse the repository at this point in the history
…ated
  • Loading branch information
ferrerojosh committed Apr 12, 2021
1 parent 5b34b52 commit 244a109
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 5 deletions.
3 changes: 2 additions & 1 deletion example/src/app.controller.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Controller, Get } from '@nestjs/common';
import { AuthenticatedUser } from 'nest-keycloak-connect';
import { AuthenticatedUser, Public } from 'nest-keycloak-connect';

@Controller()
export class AppController {
@Get()
@Public(false)
getHello(
@AuthenticatedUser()
user: any,
Expand Down
3 changes: 2 additions & 1 deletion example/src/product/product/product.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ export class ProductController {
constructor(private service: ProductService) {}

@Get()
@Public()
@Public(false)
@Scopes('View')
async findAll() {
return await this.service.findAll();
}
Expand Down
13 changes: 10 additions & 3 deletions src/guards/auth.guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ export class AuthGuard implements CanActivate {
this.extractJwt(request.headers);
const isInvalidJwt = jwt === null || jwt === undefined;

// Invalid JWT, but skipAuth = false, allow fallback
if (isInvalidJwt && !skipAuth) {
this.logger.verbose('Invalid JWT, skipAuth disabled, allowed for fallback');
return true;
}

// No jwt token given, immediate return
if (isInvalidJwt) {
this.logger.verbose('Invalid JWT, unauthorized');
Expand Down Expand Up @@ -89,15 +95,16 @@ export class AuthGuard implements CanActivate {

private extractJwt(headers: { [key: string]: string }) {
if (headers && !headers.authorization) {
throw new UnauthorizedException();
this.logger.verbose(`No authorization header`);
return null;
}

const auth = headers.authorization.split(' ');

// We only allow bearer
if (auth[0].toLowerCase() !== 'bearer') {
this.logger.verbose(`No bearer header, unauthorized`);
throw new UnauthorizedException();
this.logger.verbose(`No bearer header`);
return null;
}

return auth[1];
Expand Down
11 changes: 11 additions & 0 deletions src/guards/resource.guard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { KEYCLOAK_INSTANCE, KEYCLOAK_LOGGER } from '../constants';
import { META_RESOURCE } from '../decorators/resource.decorator';
import { META_SCOPES } from '../decorators/scopes.decorator';
import { extractRequest } from '../util';
import { META_UNPROTECTED } from '../decorators/unprotected.decorator';

/**
* This adds a resource guard, which is permissive.
Expand All @@ -37,6 +38,10 @@ export class ResourceGuard implements CanActivate {
META_SCOPES,
context.getHandler(),
);
const isUnprotected = this.reflector.getAllAndOverride<boolean>(
META_UNPROTECTED,
[context.getClass(), context.getHandler()],
);

// No resource given, since we are permissive, allow
if (!resource) {
Expand All @@ -61,8 +66,14 @@ export class ResourceGuard implements CanActivate {
// Extract request/response
const [request, response] = extractRequest(context);

if(!request.user && isUnprotected) {
this.logger.verbose(`Route has no user, and is public, allowed`);
return true;
}

const user = request.user?.preferred_username ?? 'user';


const enforcerFn = createEnforcerContext(request, response);
const isAllowed = await enforcerFn(this.keycloak, permissions);

Expand Down

0 comments on commit 244a109

Please sign in to comment.