Fix custom strategy name not being set in session (#192)

* Update outdated JSdoc comments and fix types

* .authenticate passes registered strategy name for setting in session

* name should not be optional

src/authenticator.ts

@@ -27,14 +27,14 @@ export class Authenticator<User = unknown> {
    */
   private strategies = new Map<string, Strategy<User, never>>();
 
-  public readonly sessionKey: NonNullable<AuthenticateOptions["sessionKey"]>;
+  public readonly sessionKey: NonNullable<AuthenticatorOptions["sessionKey"]>;
   public readonly sessionErrorKey: NonNullable<
-    AuthenticateOptions["sessionErrorKey"]
+    AuthenticatorOptions["sessionErrorKey"]
   >;
   public readonly sessionStrategyKey: NonNullable<
     AuthenticateOptions["sessionStrategyKey"]
   >;
-  private readonly throwOnError: AuthenticateOptions["throwOnError"];
+  private readonly throwOnError: AuthenticatorOptions["throwOnError"];
 
   /**
    * Create a new instance of the Authenticator.
@@ -96,9 +96,6 @@ export class Authenticator<User = unknown> {
   /**
    * Call this to authenticate a request using some strategy. You pass the name
    * of the strategy you want to use and the request to authenticate.
-   * The optional callback allows you to do something with the user object
-   * before returning a new Response. In case it's not provided the strategy
-   * will return a new Response and set the user to the session.
    * @example
    * let action: ActionFunction = async ({ request }) => {
    *   let user = await authenticator.authenticate("some", request);
@@ -127,6 +124,7 @@ export class Authenticator<User = unknown> {
       {
         throwOnError: this.throwOnError,
         ...options,
+        name: strategy,
         sessionKey: this.sessionKey,
         sessionErrorKey: this.sessionErrorKey,
         sessionStrategyKey: this.sessionStrategyKey,
@@ -212,7 +210,7 @@ export class Authenticator<User = unknown> {
   async logout(
     request: Request | Session,
     options: { redirectTo: string }
-  ): Promise<void> {
+  ): Promise<never> {
     let session = isSession(request)
       ? request
       : await this.sessionStorage.getSession(request.headers.get("Cookie"));

src/strategy.ts

@@ -23,7 +23,11 @@ export interface AuthenticateOptions {
    * The key of the session used to set the strategy used to authenticate the
    * user.
    */
-  sessionStrategyKey?: string;
+  sessionStrategyKey: string;
+  /**
+   * The name used to register the strategy
+   */
+  name: string;
   /**
    * To what URL redirect in case of a successful authentication.
    * If not defined, it will return the user data.
@@ -102,7 +106,8 @@ export abstract class Strategy<User, VerifyOptions> {
   /**
    * Throw an AuthorizationError or a redirect to the failureRedirect.
    * @param message The error message to set in the session.
-   * @param session The session object to set the error in.
+   * @param request The request to get the cookie out of.
+   * @param sessionStorage The session storage to retrieve the session from.
    * @param options The strategy options.
    * @throws {AuthorizationError} If the throwOnError is set to true.
    * @throws {Response} If the failureRedirect is set or throwOnError is false.
@@ -126,7 +131,7 @@ export abstract class Strategy<User, VerifyOptions> {
 
     // if we do have a failureRedirect, we redirect to it and set the error
     // in the session errorKey
-    session.flash(options.sessionErrorKey || "auth:error", { message });
+    session.flash(options.sessionErrorKey, { message });
     throw redirect(options.failureRedirect, {
       headers: { "Set-Cookie": await sessionStorage.commitSession(session) },
     });
@@ -135,7 +140,8 @@ export abstract class Strategy<User, VerifyOptions> {
   /**
    * Returns the user data or throw a redirect to the successRedirect.
    * @param user The user data to set in the session.
-   * @param session The session object to set the user in.
+   * @param request The request to get the cookie out of.
+   * @param sessionStorage The session storage to retrieve the session from.
    * @param options The strategy options.
    * @returns {Promise<User>} The user data.
    * @throws {Response} If the successRedirect is set, it will redirect to it.
@@ -156,7 +162,7 @@ export abstract class Strategy<User, VerifyOptions> {
     // if we do have a successRedirect, we redirect to it and set the user
     // in the session sessionKey
     session.set(options.sessionKey, user);
-    session.set(options.sessionStrategyKey || "strategy", this.name);
+    session.set(options.sessionStrategyKey, options.name ?? this.name);
     throw redirect(options.successRedirect, {
       headers: { "Set-Cookie": await sessionStorage.commitSession(session) },
     });

test/authenticator.test.ts

@@ -66,7 +66,7 @@ describe(Authenticator, () => {
     );
   });
 
-  test("should store the strategy name in the session", async () => {
+  test("should store the strategy provided name in the session if no custom name provided", async () => {
     let user = { id: "123" };
     let session = await sessionStorage.getSession();
     let request = new Request("/", {
@@ -90,6 +90,30 @@ describe(Authenticator, () => {
       expect(strategy).toBe("mock");
     }
   });
+  test("should store the provided strategy name in the session", async () => {
+    let user = { id: "123" };
+    let session = await sessionStorage.getSession();
+    let request = new Request("/", {
+      headers: { Cookie: await sessionStorage.commitSession(session) },
+    });
+
+    let authenticator = new Authenticator(sessionStorage, {
+      sessionStrategyKey: "strategy-name",
+    });
+    authenticator.use(new MockStrategy(async () => user), "mock2");
+
+    try {
+      await authenticator.authenticate("mock2", request, {
+        successRedirect: "/",
+      });
+    } catch (error) {
+      if (!(error instanceof Response)) throw error;
+      let cookie = error.headers.get("Set-Cookie");
+      let responseSession = await sessionStorage.getSession(cookie);
+      let strategy = responseSession.get(authenticator.sessionStrategyKey);
+      expect(strategy).toBe("mock2");
+    }
+  });
 
   test("should redirect after logout", async () => {
     let user = { id: "123" };