Repository: solid/community-server (ts)

Issue:
Make the login page do something useful without using the authn client
The login page only works if you're in an authn session (started by `session.login`) so the OIDC library already has the relevant metadata. Potentially we can also set the correct cookie outside of that but would have to investigate if this is possible and valid OIDC.
Clarify that users need to log in through an app
We currently have 2 inbound links to the login page, but that page can only be used as part of an OIDC flow.

I would suggest to keep the login links (people will be looking for them), but then provide an explanation on the rendered login page.

@joachimvh Can we detect that we're not part of an OIDC flow in that handler?

Pull request description:
Only link to log in page while in an auth flow
Part of #880, closes #853 (sort of)

Patch:
diff --git a/src/identity/IdentityProviderHttpHandler.ts b/src/identity/IdentityProviderHttpHandler.ts
index deaeb6761..dfc65da0f 100644
--- a/src/identity/IdentityProviderHttpHandler.ts
+++ b/src/identity/IdentityProviderHttpHandler.ts
@@ -167,8 +167,9 @@ export class IdentityProviderHttpHandler extends HttpHandler {
   private async resolveRoute(request: HttpRequest, response: HttpResponse, route: InteractionRoute,
     oidcInteraction?: Interaction): Promise<void> {
     if (request.method === 'GET') {
-      // .ejs templates errors on undefined variables
-      return await this.handleTemplateResponse(response, route.viewTemplate, { errorMessage: '', prefilled: {}});
+      return await this.handleTemplateResponse(
+        response, route.viewTemplate, { errorMessage: '', prefilled: {}}, oidcInteraction,
+      );
     }
 
     if (request.method === 'POST') {
@@ -179,22 +180,33 @@ export class IdentityProviderHttpHandler extends HttpHandler {
         // Render error in the view
         const prefilled = IdpInteractionError.isInstance(error) ? error.prefilled : {};
         const errorMessage = createErrorMessage(error);
-        return await this.handleTemplateResponse(response, route.viewTemplate, { errorMessage, prefilled });
+        return await this.handleTemplateResponse(
+          response, route.viewTemplate, { errorMessage, prefilled }, oidcInteraction,
+        );
       }
 
       if (result.type === 'complete') {
+        if (!oidcInteraction) {
+          // Once https://github.com/solid/community-server/pull/898 is merged
+          // we want to assign an error code here to have a more thorough explanation
+          throw new BadRequestHttpError(
+            'This action can only be executed as part of an authentication flow. It should not be used directly.',
+          );
+        }
         return await this.interactionCompleter.handleSafe({ ...result.details, request, response });
       }
       if (result.type === 'response' && route.responseTemplate) {
-        return await this.handleTemplateResponse(response, route.responseTemplate, result.details);
+        return await this.handleTemplateResponse(response, route.responseTemplate, result.details, oidcInteraction);
       }
     }
     throw new BadRequestHttpError(`Unsupported request: ${request.method} ${request.url}`);
   }
 
-  private async handleTemplateResponse(response: HttpResponse, templateFile: string, contents?: NodeJS.Dict<any>):
-  Promise<void> {
-    await this.templateHandler.handleSafe({ response, templateFile, contents: contents ?? {}});
+  private async handleTemplateResponse(response: HttpResponse, templateFile: string, data?: NodeJS.Dict<any>,
+    oidcInteraction?: Interaction): Promise<void> {
+    const contents = data ?? {};
+    contents.authenticating = Boolean(oidcInteraction);
+    await this.templateHandler.handleSafe({ response, templateFile, contents });
   }
 
   /**
diff --git a/templates/identity/email-password/register-response.html.ejs b/templates/identity/email-password/register-response.html.ejs
index bf5246af4..b9ce94f1b 100644
--- a/templates/identity/email-password/register-response.html.ejs
+++ b/templates/identity/email-password/register-response.html.ejs
@@ -26,7 +26,11 @@
   <h2>Your new account</h2>
   <p>
     Via your email address <em><%= email %></em>,
-    this server lets you <a href="./login">log in</a> to Solid apps
+    <% if (authenticating) { %>
+      you can now <a href="./login">log in</a>
+    <% } else { %>
+      this server lets you log in to Solid apps
+    <% } %>
     with your WebID <a href="<%= webId %>" class="link"><%= webId %></a>
   </p>
   <% if (!createWebId) { %>


Test patch:
diff --git a/test/unit/identity/IdentityProviderHttpHandler.test.ts b/test/unit/identity/IdentityProviderHttpHandler.test.ts
index eec635f49..a3f3be418 100644
--- a/test/unit/identity/IdentityProviderHttpHandler.test.ts
+++ b/test/unit/identity/IdentityProviderHttpHandler.test.ts
@@ -83,9 +83,9 @@ describe('An IdentityProviderHttpHandler', (): void => {
     request.url = '/idp/routeResponse';
     await expect(handler.handle({ request, response })).resolves.toBeUndefined();
     expect(templateHandler.handleSafe).toHaveBeenCalledTimes(1);
-    expect(templateHandler.handleSafe).toHaveBeenLastCalledWith(
-      { response, templateFile: routes.response.viewTemplate, contents: { errorMessage: '', prefilled: {}}},
-    );
+    expect(templateHandler.handleSafe).toHaveBeenLastCalledWith({ response,
+      templateFile: routes.response.viewTemplate,
+      contents: { errorMessage: '', prefilled: {}, authenticating: false }});
   });
 
   it('calls the templateHandler for InteractionResponseResults.', async(): Promise<void> => {
@@ -96,29 +96,51 @@ describe('An IdentityProviderHttpHandler', (): void => {
     expect(routes.response.handler.handleSafe).toHaveBeenLastCalledWith({ request });
     expect(templateHandler.handleSafe).toHaveBeenCalledTimes(1);
     expect(templateHandler.handleSafe).toHaveBeenLastCalledWith(
-      { response, templateFile: routes.response.responseTemplate, contents: { key: 'val' }},
+      { response, templateFile: routes.response.responseTemplate, contents: { key: 'val', authenticating: false }},
     );
   });
 
-  it('supports InteractionResponseResults without details.', async(): Promise<void> => {
+  it('indicates to the templates if the request is part of an auth flow.', async(): Promise<void> => {
     request.url = '/idp/routeResponse';
     request.method = 'POST';
+    const oidcInteraction = { session: { accountId: 'account' }} as any;
+    provider.interactionDetails.mockResolvedValueOnce(oidcInteraction);
     (routes.response.handler as jest.Mocked<InteractionHandler>).handleSafe.mockResolvedValueOnce({ type: 'response' });
     await expect(handler.handle({ request, response })).resolves.toBeUndefined();
     expect(routes.response.handler.handleSafe).toHaveBeenCalledTimes(1);
-    expect(routes.response.handler.handleSafe).toHaveBeenLastCalledWith({ request });
+    expect(routes.response.handler.handleSafe).toHaveBeenLastCalledWith({ request, oidcInteraction });
     expect(templateHandler.handleSafe).toHaveBeenCalledTimes(1);
     expect(templateHandler.handleSafe).toHaveBeenLastCalledWith(
-      { response, templateFile: routes.response.responseTemplate, contents: {}},
+      { response, templateFile: routes.response.responseTemplate, contents: { authenticating: true }},
     );
   });
 
-  it('calls the interactionCompleter for InteractionCompleteResults.', async(): Promise<void> => {
+  it('errors for InteractionCompleteResults if no oidcInteraction is defined.', async(): Promise<void> => {
     request.url = '/idp/routeComplete';
     request.method = 'POST';
+    errorHandler.handleSafe.mockResolvedValueOnce({ statusCode: 400 });
     await expect(handler.handle({ request, response })).resolves.toBeUndefined();
     expect(routes.complete.handler.handleSafe).toHaveBeenCalledTimes(1);
     expect(routes.complete.handler.handleSafe).toHaveBeenLastCalledWith({ request });
+    expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(0);
+
+    const error = new BadRequestHttpError(
+      'This action can only be executed as part of an authentication flow. It should not be used directly.',
+    );
+    expect(errorHandler.handleSafe).toHaveBeenCalledTimes(1);
+    expect(errorHandler.handleSafe).toHaveBeenLastCalledWith({ error, preferences: { type: { 'text/plain': 1 }}});
+    expect(responseWriter.handleSafe).toHaveBeenCalledTimes(1);
+    expect(responseWriter.handleSafe).toHaveBeenLastCalledWith({ response, result: { statusCode: 400 }});
+  });
+
+  it('calls the interactionCompleter for InteractionCompleteResults.', async(): Promise<void> => {
+    request.url = '/idp/routeComplete';
+    request.method = 'POST';
+    const oidcInteraction = { session: { accountId: 'account' }} as any;
+    provider.interactionDetails.mockResolvedValueOnce(oidcInteraction);
+    await expect(handler.handle({ request, response })).resolves.toBeUndefined();
+    expect(routes.complete.handler.handleSafe).toHaveBeenCalledTimes(1);
+    expect(routes.complete.handler.handleSafe).toHaveBeenLastCalledWith({ request, oidcInteraction });
     expect(interactionCompleter.handleSafe).toHaveBeenCalledTimes(1);
     expect(interactionCompleter.handleSafe).toHaveBeenLastCalledWith({ request, response, webId: 'webId' });
   });
@@ -155,7 +177,8 @@ describe('An IdentityProviderHttpHandler', (): void => {
     expect(templateHandler.handleSafe).toHaveBeenLastCalledWith({
       response,
       templateFile: routes.response.viewTemplate,
-      contents: { errorMessage: 'handle error', prefilled: { name: 'name' }},
+      contents: { errorMessage: 'handle error', prefilled: { name: 'name' }, authenticating: false },
+
     });
   });
 
@@ -168,7 +191,7 @@ describe('An IdentityProviderHttpHandler', (): void => {
     expect(templateHandler.handleSafe).toHaveBeenLastCalledWith({
       response,
       templateFile: routes.response.viewTemplate,
-      contents: { errorMessage: 'handle error', prefilled: { }},
+      contents: { errorMessage: 'handle error', prefilled: {}, authenticating: false },
     });
   });