-
-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow signing in users directly through the auth token guard #251
base: 9.x
Are you sure you want to change the base?
Conversation
Like it is possible with the session guard. This makes it a lot simpler to use a non-lucid user provider for the access token guard. Otherwise controllers would have to use "authenticateAsClient" - which is meant for test purposes, or the token creation logic would have to be extracted to a separate service/provider which would then have to be made available to the controller.
Hello @jeppester I am a little confused. The Sorry, haven't been able to understand the motivation for the PR :) |
Userprovider is a private attribute on the auth guard. So there is - unless I overlooked something - no way to call "createToken" from a controller. When using lucid that is not an issue because that functionality is implemented by adding a static tokenProvider to the user model. But since we don't have a model layer, we have no meaningful place to add such functionality, and being able to call |
To clarify a bit. In our project we have a kysely auth provider that looks like this: import type { Users } from '../../database/types.js'
import { Selectable } from 'kysely'
import { type Secret } from '@adonisjs/core/helpers'
import { symbols } from '@adonisjs/auth'
import {
AccessTokensGuardUser,
AccessTokensUserProviderContract,
} from '@adonisjs/auth/types/access_tokens'
import { AccessToken } from '@adonisjs/auth/access_tokens'
import { db } from '#services/db'
export type AuthUser = Pick<Selectable<Users>, 'id' | 'name'>
type DbToken = {
id: string
userId: string
hash: string
createdAt: Date
updatedAt: Date
lastUsedAt: Date | null
expiresAt: Date
}
export class KyselyAccessTokenProvider implements AccessTokensUserProviderContract<AuthUser> {
declare [symbols.PROVIDER_REAL_USER]: AuthUser
dbRowAccessTokenAttributes(dbToken: DbToken) {
return {
identifier: dbToken.id,
tokenableId: dbToken.userId,
type: 'access_token',
prefix: 'oat_',
name: '',
hash: dbToken.hash,
abilities: ['*'],
createdAt: dbToken.createdAt,
updatedAt: dbToken.updatedAt,
lastUsedAt: dbToken.lastUsedAt,
expiresAt: dbToken.expiresAt,
}
}
async createUserForGuard(user: AuthUser): Promise<AccessTokensGuardUser<AuthUser>> {
return {
getId() {
return user.id
},
getOriginal() {
return user
},
}
}
async findById(identifier: string): Promise<AccessTokensGuardUser<AuthUser> | null> {
const user = await db()
.selectFrom('users')
.select(['id', 'name'])
.where('id', '=', identifier)
.executeTakeFirst()
/* v8 ignore next */
if (!user) return null
return this.createUserForGuard(user)
}
async createToken(
user: AuthUser,
_abilities?: string[],
options?: { name?: string; expiresIn?: string | number }
): Promise<AccessToken> {
const transientToken = AccessToken.createTransientToken(user.id, 64, options?.expiresIn || 3600)
const dbToken = await db()
.insertInto('accessTokens')
.values({
userId: transientToken.userId as string,
hash: transientToken.hash,
createdAt: new Date(),
updatedAt: new Date(),
lastUsedAt: null,
expiresAt: transientToken.expiresAt!,
})
.returningAll()
.executeTakeFirstOrThrow()
return new AccessToken({
...this.dbRowAccessTokenAttributes(dbToken),
secret: transientToken.secret,
})
}
/**
* Verify a token by its publicly shared value.
*/
async verifyToken(tokenValue: Secret<string>): Promise<AccessToken | null> {
const decodedToken = AccessToken.decode('oat_', tokenValue.release())
if (!decodedToken) return null
const dbToken = await db()
.selectFrom('accessTokens')
.selectAll()
.where('id', '=', decodedToken.identifier)
.executeTakeFirstOrThrow()
if (!dbToken) return null
// We mutate dbToken so that we can later grab all AccessToken fields from dbToken
dbToken.updatedAt = new Date()
// Update DB
await db()
.updateTable('accessTokens')
.where('id', '=', dbToken.id)
.set('lastUsedAt', dbToken.updatedAt)
.executeTakeFirstOrThrow()
/**
* Create access token instance
*/
const accessToken = new AccessToken(this.dbRowAccessTokenAttributes(dbToken))
/**
* Ensure the token secret matches the token hash
*/
if (!accessToken.verify(decodedToken.secret) || accessToken.isExpired()) {
return null
}
return accessToken
}
} In our controllers the only way - again unless I'm missing something - to create a token is through the async signin({ auth, ... }: HttpContext) {
...
if (user) {
return await auth
.use('token')
.authenticateAsClient(user) // Will return { "headers": { "authorization": "bearer ..." } }
}
response.status(204)
} What we want is to instead be able to call: async signin({ auth, ... }: HttpContext) {
...
if (user) {
return await auth
.use('token')
.login(user) // Will return an AccessToken, which will be automatically formatted in a meaningful way
}
response.status(204)
} Like it is possible to do when using session auth. Again, I could be mistaking here, so if there is already a way to call |
Just wondering, can't you use the The reason, I have kept it out of the With that said, still curious to know what you think of using |
That would work (e.g. Also, the user provider could potentially take a bunch of options (for instance the table names, or the DB connection) in which case it would not be possible to instantiate without providing those options again, or it would have to be a singleton or provided to the DI container. I find it much simpler that any such options can be provided as part of the auth config in In general, as I see it, the purpose of the auth providers is to provide a neat and simple API for a certain type of authorization, to be utilized by handlers in middleware and controllers. Having to go around that API in order to sign in (and potentially out) a user seems like an unnecessary limitation to me, and also confusing, especially for those who've worked with the session auth provider. |
I have no issues in exposing a function to create a token from the
Because login via session is an act of modifying the HTTP current request, whereas in case of access token guard you are simply creating a record in the database and has nothing to do with the HTTP request. That is a big semantic difference, because you cannot create a login session without an HTTP request, but you should be able to create tokens without an HTTP request.
Yes, it could and I can see how that could be a problem. But this problem can be mitigated by having separate TokensProvider that is used by the UserProvider and also in other places in your app, exactly the way we are doing in case of Lucid. Now, I understand that creating these abstractions can feel overwhelming when the initial goal is to quickly get login up and running. With that said, let's get this PR merged. But I thought it will be helpful to explain my stance of these abstractions. Also, would you be up for writing an article around using Kysely for generating access tokens and authentication? I think a lot of people will find it useful and I will be happy to host it on our official blog. |
Oh yeah. Could we rename this method to |
π Linked issue
I jumped right to the "making a PR"-step, I hope that it is okay? Otherwise I'll create an issue.
β Type of change
π Description
Allow signing in users directly through the auth token guard, like it is possible to do with the session guard.
This makes it a lot simpler to use a non-lucid user provider for the access token guard, as otherwise controllers have to use "authenticateAsClient" - which is meant for test purposes, or the token creation logic would have to be extracted to a separate service/provider, which would then have to be made available to the controllers.
We hit this issues as we were building a kysely token user provider for our app project. To our surprise we could not sign in a user through the auth guard, like we are used to for session guard.
I'd like to also add a
logout
-method to the guard, but adding that means extending theAccessTokensUserProviderContract
(with aninvalidateToken
-method), which seems like a slightly bigger change. So I started with the part that creates the token.π Checklist