Decouple authentication logic to support multiple Auth Providers #72

esroyo commented 2026年03月17日 22:39:34 +01:00 (Migrated from github.com)

I was wondering if implementing authentication methods beyond Basic Auth (for instance, via GitHub) would be of general interest for the CMS. I would definitely find it useful.

The first step would likely be extracting the authentication logic out of the User class.

As far as I can see, there are two core functional requirements:

• Get the current state: Determine if the user is authenticated via session, cookie, header, etc.
• Perform authentication: Initiate the process, redirect to the login page (internal or external), etc.

Here is a proposed interface to handle this:

export interface AuthProvider {
 /**
 * Retrieves the current session from the incoming request headers.
 * Resolves to `null` if unauthenticated.
 */
 getSession(options: { headers: Headers }): Promise<AuthSession | null>;
 /**
 * Generates the appropriate Response to challenge the user to authenticate.
 * For Basic Auth, this would resolve with a 401 and WWW-Authenticate header.
 */
 getChallengeResponse(options?: { ... }): Response | Promise<Response>;
}
export interface AuthSession {
 user: UserConfiguration;
 // session: { ...data... }
}

And an example of its usage in the CMS routing:

 .path("/*", async ({ request, next, _ }) => {
 const user = new User();
 if (this.authProvider && _.join("/") !== "logout") {
 const session = await this.authProvider.getSession({
 headers: request.headers,
 });
 if (!session) {
 return this.authProvider.getChallengeResponse({
 redirectUri: request.url,
 });
 }
 // Simply sets the user config/info the provider has delivered 
 await user.logIn(session.user);
 }

Since I'm not entirely familiar with the project's internals, I'm a bit unsure if this approach aligns with the overall vision. I'd love to get your thoughts on the idea, @oscarotero, whenever you have the time—no rush!

The naming and shape proposed here are loosely based on better-auth. I have also implemented a small PoC to show what this could look like in practice while maintaining backward compatibility (see diff).

Thanks for your time!

I was wondering if implementing authentication methods beyond Basic Auth (for instance, via GitHub) would be of general interest for the CMS. I would definitely find it useful. The first step would likely be extracting the authentication logic out of the `User` class. As far as I can see, there are two core functional requirements: * **Get the current state:** Determine if the user is authenticated via session, cookie, header, etc. * **Perform authentication:** Initiate the process, redirect to the login page (internal or external), etc. Here is a proposed interface to handle this: ```ts export interface AuthProvider { /** * Retrieves the current session from the incoming request headers. * Resolves to `null` if unauthenticated. */ getSession(options: { headers: Headers }): Promise<AuthSession | null>; /** * Generates the appropriate Response to challenge the user to authenticate. * For Basic Auth, this would resolve with a 401 and WWW-Authenticate header. */ getChallengeResponse(options?: { ... }): Response | Promise<Response>; } export interface AuthSession { user: UserConfiguration; // session: { ...data... } } ``` And an example of its usage in the CMS routing: ```ts .path("/*", async ({ request, next, _ }) => { const user = new User(); if (this.authProvider && _.join("/") !== "logout") { const session = await this.authProvider.getSession({ headers: request.headers, }); if (!session) { return this.authProvider.getChallengeResponse({ redirectUri: request.url, }); } // Simply sets the user config/info the provider has delivered await user.logIn(session.user); } ``` Since I'm not entirely familiar with the project's internals, I'm a bit unsure if this approach aligns with the overall vision. I'd love to get your thoughts on the idea, @oscarotero, whenever you have the time—no rush! The naming and shape proposed here are loosely based on [better-auth](https://better-auth.com/docs/basic-usage#session). I have also implemented a small PoC to show what this could look like in practice while maintaining backward compatibility ([see diff](https://github.com/lumeland/cms/compare/main...esroyo:cms:feat/auth-provider-interface?expand=1)). Thanks for your time!

oscarotero commented 2026年03月18日 18:24:37 +01:00

LumeCMS uses Basic Auth because it just work. In most cases, I only need a couple of profiles (one with more permissions than other), for example "admin" and "content". And the same credentials are used by serveral people. It's also easier to configure because the users permissions can be defined in the _cms.ts file.

Adding support for external Auth Providers could introduce more complexity, not only to add new users but to manage them. And I guess it requires some kind of backend storage to save tokens and that stuff.

Anyway, I'm not against to this, in fact I left the door open to add other auth providers (that's the reason of the "method" property), so if it can be implemented in a way that doesn't break the existing rules, I'm okay with that. For example, you proposal to use a generic interface looks fine.

LumeCMS uses Basic Auth because it just work. In most cases, I only need a couple of profiles (one with more permissions than other), for example "admin" and "content". And the same credentials are used by serveral people. It's also easier to configure because the users permissions can be defined in the _cms.ts file. Adding support for external Auth Providers could introduce more complexity, not only to add new users but to manage them. And I guess it requires some kind of backend storage to save tokens and that stuff. Anyway, I'm not against to this, in fact I left the door open to add other auth providers (that's the reason of [the "method" property](https://github.com/lumeland/cms/blob/31c59bba6094f9a91dca45bab8705dcfab034143/core/cms.ts#L42)), so if it can be implemented in a way that doesn't break the existing rules, I'm okay with that. For example, you proposal to use a generic interface looks fine.

esroyo commented 2026年03月21日 00:59:04 +01:00 (Migrated from github.com)

@oscarotero I strongly agree with your thoughts on complexity; one must take extra care to ensure it doesn't leak into the rest of the system. I also completely understand that this isn't a priority for you right now, given that Basic Auth is sufficient. That makes total sense. However, in case you are in the mood to chat about the topic, I wanted to leave some thoughts.

Regarding the need for a backend or dashboard, I'm not aiming for that at all. I was thinking of implementing something stateless and continuing to manage user permissions through the current static map:

cms.auth({
 github_user1: { // we still need to be explicit about who we accept (authorization, static)
 password: "", // but password can be empty or ignored (authentication, third-party provider)
 permissions: { /* ... */ }, // (authorization, static)
 }, /* ... */
});

The AuthProvider interface covers the most basic needs for a start. Where I struggled the most, and where I'd need more guidance, is how to integrate the provider with the CMS.

At first, I thought of making the cms.auth(users) method polymorphic by allowing it to accept an instance of the provider:

const authProvider = new AuthFoo({ users: ... });
cms.auth(authProvider);

This might not look too bad, but the setup boilerplate starts to get too large later on. The types accepted by auth() are too different for my taste. Additionally, for something like OAuth, we need to hook into the routing. Should the Cms instance itself be injected into AuthFoo on construction, or should we establish a connection between the provider and the CMS?

The best approach I have come up with is something extremely similar to the existing plugin pattern. It's just a little more formal—designed to return an instance rather than register itself:

type AuthOptions = {
 factory: (c: Cms) => AuthProvider;
 method: string;
 users: Record<string, string | UserConfiguration>;
}

Given that shape, the factory and method could be passed after the users object to cms.auth(), making the overload pretty manageable:

 auth(
 users: Record<string, string | UserConfiguration>,
 authOptions?: Omit<AuthOptions, "users">,
 ): this

Although it would be technically possible to pass a single parameter with the shape of AuthOptions, I don't feel comfortable with that level of duck-typing.

In any case, I think this strikes a good balance between maintaining backward compatibility and providing ease of opt-in. Usage would look something like this:

cms.auth({ 
 admin: '1234',
}, { 
 method: 'github',
 factory: AuthGitHub.createFactory({ clientId, clientSecret, clientType: "github-app" }),
});

It's definitely not the most beautiful code in the world, but it could pass. I have successfully tested the idea locally with Octokit's OAuthApp, but in my branch I've only included the Basic Auth replacement so there's less of a diff to review at a glance.

I hope I'm not taking up too much of your time with this. Let me know if you come up with any other ideas for integrating it.

Thanks!

@oscarotero I strongly agree with your thoughts on complexity; one must take extra care to ensure it doesn't leak into the rest of the system. I also completely understand that this isn't a priority for you right now, given that Basic Auth is sufficient. That makes total sense. However, in case you are in the mood to chat about the topic, I wanted to leave some thoughts. Regarding the need for a backend or dashboard, I'm not aiming for that at all. I was thinking of implementing something [stateless](https://better-auth.com/docs/concepts/session-management#session-caching) and continuing to manage user permissions through the current static map: ```ts cms.auth({ github_user1: { // we still need to be explicit about who we accept (authorization, static) password: "", // but password can be empty or ignored (authentication, third-party provider) permissions: { /* ... */ }, // (authorization, static) }, /* ... */ }); ``` The `AuthProvider` interface covers the most basic needs for a start. Where I struggled the most, and where I'd need more guidance, is how to integrate the provider with the CMS. At first, I thought of making the `cms.auth(users)` method polymorphic by allowing it to accept an instance of the provider: ```ts const authProvider = new AuthFoo({ users: ... }); cms.auth(authProvider); ``` This might not look too bad, but the setup boilerplate starts to get too large later on. The types accepted by `auth()` are too different for my taste. Additionally, for something like OAuth, we need to hook into the routing. Should the `Cms` instance itself be injected into `AuthFoo` on construction, or should we establish a connection between the provider and the CMS? The best approach I have come up with is something extremely similar to the existing plugin pattern. It's just a little more formal—designed to return an instance rather than register itself: ```ts type AuthOptions = { factory: (c: Cms) => AuthProvider; method: string; users: Record<string, string | UserConfiguration>; } ``` Given that shape, the `factory` and `method` could be passed after the `users` object to `cms.auth()`, making the overload pretty manageable: ```ts auth( users: Record<string, string | UserConfiguration>, authOptions?: Omit<AuthOptions, "users">, ): this ``` Although it would be technically possible to pass a single parameter with the shape of `AuthOptions`, I don't feel comfortable with that level of duck-typing. In any case, I think this strikes a good balance between maintaining backward compatibility and providing ease of opt-in. Usage would look something like this: ```ts cms.auth({ admin: '1234', }, { method: 'github', factory: AuthGitHub.createFactory({ clientId, clientSecret, clientType: "github-app" }), }); ``` It's definitely not the most beautiful code in the world, but it could pass. I have successfully tested the idea locally with Octokit's `OAuthApp`, but [in my branch](https://github.com/lumeland/cms/compare/main...esroyo:cms:feat/auth-provider-interface?expand=1) I've only included the Basic Auth replacement so there's less of a diff to review at a glance. I hope I'm not taking up too much of your time with this. Let me know if you come up with any other ideas for integrating it. Thanks!

oscarotero commented 2026年03月21日 15:18:51 +01:00

Okay, let me know what do you think about this idea:

• As you know, the auth system in Lume CMS consists of only two variables: method (which is a string that only accepts the value "basic" and the users object with the configuration.
• Lume CMS uses already the concept of "strings vs objects/instances" in different places. The strings are used for brevity when you're okay with the default values. And object/instances when you need more customization.
• As an example, you can define a filesystem store as cms.storage("src", "my-folder") (a string) and internally is resolved to cms.storage("src", Fs.create(Deno.cwd() + "/my-folder")) (an instance). To use a different storage method (sqlite, GitHub, etc) you need to pass the instance.
• Another example is the fields definition: "title: text" string is resolved to {type: "text", name: "title" } object.
• I think the method option of the auth could work in the same way. The string "basic" would be resolved internally to an instance of BasicAuth class (for example). Other methods could be passed as instances.

Lume CMS allows to pass the auth configuration to the constructor:

const cms = lumeCMS({
 auth: {
 method: "basic",
 users: {}
 }
})

This could be the same as:

const cms = lumeCMS({
 auth: {
 method: BasicAuth.create(),
 users: {}
 }
})

And allow other methods:

const cms = lumeCMS({
 auth: {
 method: GitHub.create({ /* options */ }),
 users: {}
 }
})

The function cms.auth() allows to configure the users but not the method. The motivation to create this function is to allow to extend the configuration of the cms after the instantation, which is useful to enable or disable auth in different environments (local vs prod):

if (production) {
 cms.auth({
 "user": "password",
 });
}

The abscense of the method argument is for brevity and because there is only one option (for now). So, your idea to extend this function to accept a second argument with the auth provider is okay. But probably we don't need the method and factory, only the provider instance:

// These are equivalents
cms.auth(users);
cms.auth(users, "basic");
cms.auth(users, BasicAuth.create());
// Use a different provider
cms.auth(users, GitHub.create( /* options */ ));

Internally, users and auth providers should be independent. Lume CMS only need the current user, but the way it's validated is up to the auth provider. Currently the auth logic is coupled to the User instance (an empty user is created and then it tries to authenticate). I think this should be changed for something like this:

const authProvider = options.auth?.method;
const users = options.auth?.users;
let user: User;
if (authProvider && _.join("/") !== "logout") {
 const name = authProvider.getUsername(request, users);
 if (name) {
 user = new User(users[name]);
 } else {
 return authProvider.login(request);
 }
} else {
 user = new User(); // Anonimous user if auth is disabled.
}

We also need a method to logout which could be something like:

return authProvider.logout(request);

So, do you think an interface like this could work?

interface AuthProvider {
 getUsername(request: Request, users: Record<string, UserConfiguration>): string;
 login(request: Request): Response;
 logout(request: Request): Response;
}

Note, this is just an idea of interface, but I'm okay with other shape if you think it makes more sense (tbh, I'm not an expert on the matter).

Okay, let me know what do you think about this idea: - As you know, the auth system in Lume CMS consists of only two variables: `method` (which is a string that only accepts the value "basic" and the users object with the configuration. - Lume CMS uses already the concept of "strings vs objects/instances" in different places. The strings are used for brevity when you're okay with the default values. And object/instances when you need more customization. - As an example, you can define a filesystem store as `cms.storage("src", "my-folder")` (a string) and internally is resolved to `cms.storage("src", Fs.create(Deno.cwd() + "/my-folder"))` (an instance). To use a different storage method (sqlite, GitHub, etc) you need to pass the instance. - Another example is the fields definition: `"title: text"` string is resolved to `{type: "text", name: "title" }` object. - I think the `method` option of the auth could work in the same way. The string "basic" would be resolved internally to an instance of `BasicAuth` class (for example). Other methods could be passed as instances. Lume CMS allows to pass the auth configuration to the constructor: ```js const cms = lumeCMS({ auth: { method: "basic", users: {} } }) ``` This could be the same as: ```js const cms = lumeCMS({ auth: { method: BasicAuth.create(), users: {} } }) ``` And allow other methods: ```js const cms = lumeCMS({ auth: { method: GitHub.create({ /* options */ }), users: {} } }) ``` The function `cms.auth()` allows to configure the users but not the method. The motivation to create this function is to allow to extend the configuration of the cms after the instantation, which is useful to enable or disable auth in different environments (local vs prod): ```js if (production) { cms.auth({ "user": "password", }); } ``` The abscense of the `method` argument is for brevity and because there is only one option (for now). So, your idea to extend this function to accept a second argument with the auth provider is okay. But probably we don't need the method and factory, only the provider instance: ```js // These are equivalents cms.auth(users); cms.auth(users, "basic"); cms.auth(users, BasicAuth.create()); // Use a different provider cms.auth(users, GitHub.create( /* options */ )); ``` Internally, users and auth providers should be independent. Lume CMS only need the current user, but the way it's validated is up to the auth provider. Currently the auth logic is [coupled to the User instance](https://github.com/lumeland/cms/blob/b10da0d03b15944a1d864981929f2303da93c5b8/core/routes/main.ts#L57) (an empty user is created and then it tries to authenticate). I think this should be changed for something like this: ```js const authProvider = options.auth?.method; const users = options.auth?.users; let user: User; if (authProvider && _.join("/") !== "logout") { const name = authProvider.getUsername(request, users); if (name) { user = new User(users[name]); } else { return authProvider.login(request); } } else { user = new User(); // Anonimous user if auth is disabled. } ``` We also need a method [to logout](https://github.com/lumeland/cms/blob/b10da0d03b15944a1d864981929f2303da93c5b8/core/routes/main.ts#L90) which could be something like: ```js return authProvider.logout(request); ``` So, do you think an interface like this could work? ```ts interface AuthProvider { getUsername(request: Request, users: Record<string, UserConfiguration>): string; login(request: Request): Response; logout(request: Request): Response; } ``` Note, this is just an idea of interface, but I'm okay with other shape if you think it makes more sense (tbh, I'm not an expert on the matter).

oscarotero commented 2026年03月24日 20:01:38 +01:00

Hi @esroyo
I just added the AuthProvider interface. You can see an example with the Basic method here: https://github.com/lumeland/cms/blob/main/auth/Basic.ts

Can you confirm if this works for you?

Hi @esroyo I just added the `AuthProvider` interface. You can see an example with the `Basic` method here: https://github.com/lumeland/cms/blob/main/auth/Basic.ts Can you confirm if this works for you?

esroyo commented 2026年03月24日 23:13:47 +01:00 (Migrated from github.com)

@oscarotero wow, that was fast. Thanks so much!

I really like the idea of using method for the provider. It is convenient and consistent with existing patterns. Regarding the interface, I'm sure it will be useful in the shape you implemented.

Another thing that will certainly be needed is the ability to "plug into" the router (for OAuth handshaking, or to serve a "login page").

That was the reason I initially thought of the "plugin pattern", to be able to receive the Cms and monkey-patch the Cms.fetch function. Yes, definitively "not elegant" 😄

Now, with the auth provider being a standalone module that knows nothing about the Cms, we will probably need to expose a "handler" on the provider so the Cms can configure it as part of the routing. This is similar to how better-auth integrates with Astro:

/** The interface to implement different auth providers */
export interface AuthProvider {
 getUsername(
 request: Request,
 users: Map<string, UserConfiguration>,
 ): string | undefined;
 login(request: Request): Response;
 logout(request: Request): Response;
+ /** It could be called `handler` or `fetch` maybe? */
+ fetch?(request: Request): Response | Promise<Response>;
}

In our case, it could simplify things to assume by convention that the route prefix /api/auth/* is handled by AuthProvider['fetch'] (if it exists). That way, there is no need for the developer to "declare" and hand off the route paths beforehand.

What do you think? Might that be too limiting? Would it be preferable to expose both the handler and the routes it needs to handle?

@oscarotero wow, that was fast. Thanks so much! I really like the idea of using `method` for the provider. It is convenient and consistent with existing patterns. Regarding the interface, I'm sure it will be useful in the shape you implemented. Another thing that will certainly be needed is the ability to "plug into" the router (for OAuth handshaking, or to serve a "login page"). That was the reason I initially thought of the "plugin pattern", to be able to receive the `Cms` and monkey-patch the `Cms.fetch` function. Yes, definitively "not elegant" 😄 Now, with the auth provider being a standalone module that knows nothing about the `Cms`, we will probably need to expose a "handler" on the provider so the `Cms` can configure it as part of the routing. This is similar to how [better-auth integrates with Astro](https://better-auth.com/docs/integrations/astro#mount-the-handler): ```diff /** The interface to implement different auth providers */ export interface AuthProvider { getUsername( request: Request, users: Map<string, UserConfiguration>, ): string | undefined; login(request: Request): Response; logout(request: Request): Response; + /** It could be called `handler` or `fetch` maybe? */ + fetch?(request: Request): Response | Promise<Response>; } ``` In our case, it could simplify things to assume by convention that the route prefix `/api/auth/*` is handled by `AuthProvider['fetch']` (if it exists). That way, there is no need for the developer to "declare" and hand off the route paths beforehand. What do you think? Might that be too limiting? Would it be preferable to expose both the handler and the routes it needs to handle?

oscarotero commented 2026年03月25日 10:34:39 +01:00

@esroyo my idea is that login can do the same as fetch. Maybe we can rename it to handle or fetch if it makes more sense and change the type to allow returning a Promise. The logic is: we try to get the username and if we can't, run login().
So, technically you could do something like this (I didn't try):

export class MyLogin extends AuthProvider {
 getUsername() {}
 logout() {}
 login(request): Response {
 return new Router()
 .get("/", () => Response.redirect("/auth/login"),
 .get("/auth/login", () => "<form method="post">...</form>",
 .post("/auth/login", () => ...,
 .fetch(request);
 }
}

What do you think?

@esroyo my idea is that `login` can do the same as `fetch`. Maybe we can rename it to `handle` or `fetch` if it makes more sense and change the type to allow returning a Promise. The logic is: [we try to get the username and if we can't, run login()](https://github.com/lumeland/cms/blob/5c7e5b075c81d10891a6cf953ba1bfd7fa2e2cb8/core/routes/main.ts#L56-L65). So, technically you could do something like this (I didn't try): ```js export class MyLogin extends AuthProvider { getUsername() {} logout() {} login(request): Response { return new Router() .get("/", () => Response.redirect("/auth/login"), .get("/auth/login", () => "<form method="post">...</form>", .post("/auth/login", () => ..., .fetch(request); } } ``` What do you think?

esroyo commented 2026年03月25日 13:32:10 +01:00 (Migrated from github.com)

@oscarotero probably that could work, but it is mixing two different concerns.

On one side, you have the active login flow (e.g., Clerk's redirectToSignIn(), BetterAuth's signIn(), or Octokit's OAuth flow getWebFlowAuthorizationUrl()). On the other side, you have the auth infrastructure routes, which should be state-agnostic. Think of something like an /auth/status or /auth/webhook/invalidate endpoint, these need to be able to receive requests without redirecting the caller to "Please login".

By exposing a fetch handler instead, the provider can manage its own route namespace cleanly:

export class MyLogin extends AuthProvider {
 getUsername() {}
 logout() {}
 
 // The active user flow
 login(): Response {
 return Response.redirect("/auth/login");
 }
 
 // The state-agnostic infrastructure routes (e.g., mounted to /auth/*)
 fetch(request): Response {
 return new Router()
 .get("/login", () => new Response('<form method="post">...</form>'))
 .get("/status", () => Response.json({ isLogged: !!cms.isLogged() }))
 .post("/webhook", () => { /* handle token invalidation */ })
 .fetch(request);
 }
}

These are just quick examples, but hopefully illustrate the points.

@oscarotero probably that _could_ work, but it is mixing two different concerns. On one side, you have the active login flow (e.g., Clerk's `redirectToSignIn()`, BetterAuth's `signIn()`, or Octokit's OAuth flow `getWebFlowAuthorizationUrl()`). On the other side, you have the auth infrastructure routes, which should be state-agnostic. Think of something like an `/auth/status` or `/auth/webhook/invalidate` endpoint, these need to be able to receive requests without redirecting the caller to "Please login". By exposing a `fetch` handler instead, the provider can manage its own route namespace cleanly: ```ts export class MyLogin extends AuthProvider { getUsername() {} logout() {} // The active user flow login(): Response { return Response.redirect("/auth/login"); } // The state-agnostic infrastructure routes (e.g., mounted to /auth/*) fetch(request): Response { return new Router() .get("/login", () => new Response('<form method="post">...</form>')) .get("/status", () => Response.json({ isLogged: !!cms.isLogged() })) .post("/webhook", () => { /* handle token invalidation */ }) .fetch(request); } } ``` These are just quick examples, but hopefully illustrate the points.

oscarotero commented 2026年03月25日 14:15:03 +01:00

Ok, got it.

I added the fetch method to the interface.
You can see how it's integrated with the CMS here: github.com/lumeland/cms@91eebfd9f3/core/routes/main.ts (L50-L58)

All requests to /path are managed by the authProvider.
Let me know if it works for you.

Ok, got it. I added the `fetch` method to the interface. You can see how it's integrated with the CMS here: https://github.com/lumeland/cms/blob/91eebfd9f3db8562b648c0a5c5a0161b1297174e/core/routes/main.ts#L50-L58 All requests to `/path` are managed by the authProvider. Let me know if it works for you.

esroyo commented 2026年03月25日 14:42:56 +01:00 (Migrated from github.com)

At first glance, it looks great, @oscarotero. I will update my local GitHub PoC tonight and report back. 👍 Thanks!

At first glance, it looks great, @oscarotero. I will update my local GitHub PoC tonight and report back. 👍 Thanks!

esroyo commented 2026年03月26日 02:38:32 +01:00 (Migrated from github.com)

@oscarotero, this is mostly fine. I just needed to convert getUsername to async (it may need to make a validation request, or in my case, use crypto.subtle). In any case, making all of these async is future-proof: here is the diff with the small tweaks.

I have successfully tested the GitHub OAuth web flow with the new code: here you can see the provider.

The only unexpected behavior was a redirect loop:

• If you log in via GitHub with a username that is not in the static auth map,
• getUsername can read your credentials (meaning you are authenticated),
• but you are not authorized (missing from the lumeCMS map).
• Therefore, getUsername fails to return a valid user, which triggers a redirect back to the login page, creating an infinite loop.

To prevent this, I'm currently throwing an error in the provider. I haven't figured out a better solution ATM.

@oscarotero, this is mostly fine. I just needed to convert getUsername to async (it may need to make a validation request, or in my case, use crypto.subtle). In any case, making all of these async is future-proof: [here is the diff with the small tweaks](https://github.com/lumeland/cms/compare/main...esroyo:cms:a4e2448ec98620f13fe651c3ded7ff4798a287bc?expand=1). I have successfully tested the GitHub OAuth web flow with the new code: [here you can see the provider](https://github.com/esroyo/cms/blob/main/auth/github.ts). The only unexpected behavior was a redirect loop: - If you log in via GitHub with a username that is not in the static auth map, - getUsername can read your credentials (meaning you are authenticated), - but you are not authorized (missing from the lumeCMS map). - Therefore, getUsername fails to return a valid user, which triggers a redirect back to the login page, creating an infinite loop. To prevent this, [I'm currently throwing an error](https://github.com/esroyo/cms/blob/main/auth/github.ts#L194) in the provider. I haven't figured out a better solution ATM.

oscarotero commented 2026年03月26日 09:55:44 +01:00

Great. Feel free to open a PR to make all methods async.
Regarding the redirect loop, I'm thinking of this:

• The getUsername function could return the username, even if it's not available in the users map.
• And here, instead of assuming that the username is already defined in the map, we check it and show a error of invalid user if it's not, with a button to login again.
• This would work the same for all auth methods.

Great. Feel free to open a PR to make all methods async. Regarding the redirect loop, I'm thinking of this: - The `getUsername` function could return the username, even if it's not available in the users map. - [And here](https://github.com/lumeland/cms/blob/8aea1711dd5a9806aea9757bc774da77fce9a627/core/routes/main.ts#L69), instead of assuming that the username is already defined in the map, we check it and show a error of invalid user if it's not, with a button to login again. - This would work the same for all auth methods.

esroyo commented 2026年03月26日 13:51:57 +01:00 (Migrated from github.com)

If I understood correctly, this would render a 403 template, similar to a generic NotFound or Error page?

 // Authentication
 if (options.authMethod && options.users.size) {
 const username = options.authMethod.getUsername(
 request,
 options.users,
 );
 if (username) {
 const config = options.users.get(username)!;
+ if (!config) return new Response("Forbidden", { status: 403 }); // TODO: a nice template
 user = new User(username, config);
 } else {
 return options.authMethod.login(request);
 }
 } else {
 user = new User(); // anonymous user
 }

---

403 - Unauthorized User

Your identity provider verified your account, but you are not registered in our system. If you believe this is an error, please reach out to the site maintainer.

<LoginButton>Log out / Switch accounts</LoginButton>

---

That would probably change the getUsername contract, right?
It removes the responsibility to match/authorize from the provider. Just authenticate (in their own realm).

export interface AuthProvider {
- getUsername(request: Request, users: Map<string, UserConfiguration>): string | undefined | Promise<string | undefined>;
+ getUsername(request: Request): string | undefined | Promise<string | undefined>;

Overall seems like a great idea @oscarotero.
Much better UX than slapping the Login prompt or a redirect in your face without prior warning :)

If I understood correctly, this would render a 403 template, similar to a generic NotFound or Error page? ```diff // Authentication if (options.authMethod && options.users.size) { const username = options.authMethod.getUsername( request, options.users, ); if (username) { const config = options.users.get(username)!; + if (!config) return new Response("Forbidden", { status: 403 }); // TODO: a nice template user = new User(username, config); } else { return options.authMethod.login(request); } } else { user = new User(); // anonymous user } ``` --- > 403 - Unauthorized User > > Your identity provider verified your account, but you are not registered in our system. If you believe this is an error, please reach out to the site maintainer. > > &lt;LoginButton&gt;Log out / Switch accounts&lt;/LoginButton&gt; --- That would probably change the `getUsername` contract, right? It removes the responsibility to match/authorize from the provider. Just authenticate (in their own realm). ```diff export interface AuthProvider { - getUsername(request: Request, users: Map<string, UserConfiguration>): string | undefined | Promise<string | undefined>; + getUsername(request: Request): string | undefined | Promise<string | undefined>; ``` Overall seems like a _great_ idea @oscarotero. Much better UX than slapping the Login prompt or a redirect in your face without prior warning :)

oscarotero commented 2026年03月26日 15:39:55 +01:00

Yes, that's the idea!
Can you work on a PR so you can test it with your providers?
Then, I can work on the template for the 403 page.

Yes, that's the idea! Can you work on a PR so you can test it with your providers? Then, I can work on the template for the 403 page.

esroyo commented 2026年03月26日 15:46:33 +01:00 (Migrated from github.com)

Yes, that's the idea! Can you work on a PR so you can test it with your providers? Then, I can work on the template for the 403 page.

Fantastic! yes, I'll work on that tonight 👍

> Yes, that's the idea! Can you work on a PR so you can test it with your providers? Then, I can work on the template for the 403 page. Fantastic! yes, I'll work on that tonight 👍

esroyo commented 2026年03月26日 23:06:39 +01:00 (Migrated from github.com)

@oscarotero I've successfully tested all the flows and changes (PR). Can't wait for the template 🤟

The only detail that didn't work out was removing the users parameter from the getUsername fn. Without it, Basic Auth won't work, or it would have to return the user+password, which breaks the whole idea. I guess other providers can just ignore the users param. It's a little misleading, but I can't think of anything else.

The PR contains two commits. I'm not sure if you are willing to officially include the AuthGitHub provider yet. It can perfectly wait until it is more mature. Your call, just let me know!

@oscarotero I've successfully tested all the flows and changes ([PR](https://github.com/lumeland/cms/pull/73)). Can't wait for the template 🤟 The only detail that didn't work out was removing the users parameter from the getUsername fn. Without it, Basic Auth won't work, or it would have to return the user+password, which breaks the whole idea. I guess other providers can just ignore the users param. It's a little misleading, but I can't think of anything else. The PR contains two commits. I'm not sure if you are willing to officially include the AuthGitHub provider yet. It can perfectly wait until it is more mature. Your call, just let me know!

esroyo commented 2026年03月30日 22:14:44 +02:00 (Migrated from github.com)

I have opened a docs PR for when this is officially released: https://github.com/lumeland/lume.land/pull/248

The GitHub Auth provider is available on my main branch for future reference.

From my side, this issue can be closed.
Thanks again for your time and patience ❤️

I have opened a docs PR for when this is officially released: https://github.com/lumeland/lume.land/pull/248 The [GitHub Auth provider](https://github.com/lumeland/cms/compare/main...esroyo:cms:feat/auth-github?expand=1) is available on my main branch for future reference. From my side, this issue can be closed. Thanks again for your time and patience ❤️

oscarotero commented 2026年04月06日 21:17:21 +02:00

Released in v0.15.0, I think this can be closed.

We can include your GitHub Auth in future versions, if you're okay with that.

Released in v0.15.0, I think this can be closed. We can include your GitHub Auth in future versions, if you're okay with that.

esroyo commented 2026年04月07日 12:16:52 +02:00 (Migrated from github.com)

Absolutely @oscarotero! Thanks 🎉 👏

Absolutely @oscarotero! Thanks 🎉 👏