Allow signing in users directly through the auth token guard #251
Conversation
|
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 |
Great, I'll make the change as soon as I get the time. What is you opinion about adding an |
A quick question. What if you want to invalidate multiple tokens? Imagine, you allow users to create one or more tokens from the UI of your app and you want them to delete the selected tokens. |
I think I'd make it so that So a CRUD flow for tokens would need its own code for token deletion, which I think is sensible. Such a flow would work the same way as other CRUD flows, e.g. identifying the entity by id. |
|
Cool, that makes sense. So essentially we add support for |
Exactly. I'll update the PR as soon as I have the time to dig into it. |
|
I changed the PR so that the sign in method is called |
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.
a3dc162 to
bf8b17a
Compare
|
@thetutlage All the added code should have coverage now, and so I believe the PR is ready for review. I tried my best to folow the existing test conventions, but it felt a bit like i wrote each test for token invalidation three times: for the guard, the user provider, and the token provider. Let me know if you prefer it another way. |
|
Looks good. Can you please also open a PR for the docs? |
Yes. From a very brief look at the docs, I see two options for how to do that - both updating the token auth docs:
What is your opinion? (maybe you have another idea?) |
|
I think option 2 makes more sense. We can start by explaining that if you are using tokens to perform login/logout, then you may prefer interacting with the guard directly to issue tokens and revoke the current token via which the request was authenticated. |
🔗 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