fix: review comments

Author: deepakrkris

extensions/authentication-passport/src/__tests__/acceptance/authentication-with-passport-strategy-oauth2-adapter.acceptance.ts

@@ -24,11 +24,7 @@ import {
   expect,
   supertest,
 } from '@loopback/testlab';
-import {
-  RestApplication,
-  RestBindings,
-  Response,
-} from '@loopback/rest';
+import {RestApplication, RestBindings, Response} from '@loopback/rest';
 import {
   startApp as startMockOauth2Server,
   stopApp as stopMockOauth2Server,
@@ -177,8 +173,8 @@ export class Oauth2Controller {
   // we have modeled this as a GET endpoint
   @get('/auth/thirdparty')
   // loginToThirdParty() is the handler for '/auth/thirdparty'
-  // this method is injected with 'x-loopback-authentication-redirect-url'
-  // the value for 'x-loopback-authentication-redirect-url' is set by the passport strategy adapter
+  // this method is injected with redirect url and status
+  // the value for 'authentication.redirect.url' is set by the authentication action provider
   loginToThirdParty(
     @inject('authentication.redirect.url')
     redirectUrl: string,
@@ -187,6 +183,8 @@ export class Oauth2Controller {
     @inject(RestBindings.Http.RESPONSE)
     response: Response,
   ) {
+    // controller handles redirect
+    // and returns response object to indicate response is already handled
     response.statusCode = status || 302;
     response.setHeader('Location', redirectUrl);
     response.end();

packages/authentication/src/keys.ts

@@ -118,6 +118,16 @@ export namespace AuthenticationBindings {
 
   // Make `CURRENT_USER` the alias of SecurityBindings.USER for backward compatibility
   export const CURRENT_USER = SecurityBindings.USER;
+
+  // Redirect url for authenticating current user
+  export const AUTHENTICATION_REDIRECT_URL = BindingKey.create<string>(
+    'authentication.redirect.url',
+  );
+
+  // Authentication redirect status, usually 302 or 303, indicates a web client will redirect
+  export const AUTHENTICATION_REDIRECT_STATUS = BindingKey.create<number>(
+    'authentication.redirect.status',
+  );
 }
 
 /**

packages/authentication/src/providers/auth-action.provider.ts

@@ -59,10 +59,10 @@ export class AuthenticateActionProvider implements Provider<AuthenticateFn> {
       // bind redirection url and status as 'authentication.oauth2.redirectUrl' to context
       // controller should handle actual redirection
       this.ctx
-        .bind('authentication.redirect.url')
+        .bind(AuthenticationBindings.AUTHENTICATION_REDIRECT_URL)
         .to(redirectOptions.targetLocation);
       this.ctx
-        .bind('authentication.redirect.status')
+        .bind(AuthenticationBindings.AUTHENTICATION_REDIRECT_STATUS)
         .to(redirectOptions.statusCode);
     } else if (authResponse) {
       // if `strategy.authenticate()` returns an object of type UserProfile, set it as current user

packages/rest/src/router/redirect-route.ts

@@ -6,11 +6,7 @@
 import {OperationObject, SchemasObject} from '@loopback/openapi-v3';
 import {ResolvedRoute, RouteEntry} from '.';
 import {RequestContext} from '../request-context';
-import {
-  OperationArgs,
-  OperationRetval,
-  PathParameterValues,
-} from '../types';
+import {OperationArgs, OperationRetval, PathParameterValues} from '../types';
 
 export class RedirectRoute implements RouteEntry, ResolvedRoute {
   // ResolvedRoute API