Use syntax JWT application configuration

[Craz1k0ek]

Is your feature request related to a problem? Please describe. As far as I can tell, all application level configurations have some kind of use syntax when being configured, i.e.:

application.passwords.use(.bcrypt)
application.middleware.use(middleware, at: position)
try application.databases.use(.postgres(url: "psql://"), as: .psql)
try application.jwt.signers.use(jwks: jwks)

As I've seen at the 5.0.0 beta release, this use syntax has been removed and instead the option for add(jwks:) was added, amongst others. In my opinion this is poor API design, as the new JWT API differs from the known use syntax. It differs too much from what is known and used, whilst not being syntactically more appealing (opinionated, but sure).

Additionally, the chosen syntax can be made more uniform, by simply reinstating the use instead of the add even for the other keys, i.e.:

// 5.0.0 beta 3
func addEdDSA(key: some EdDSAKey)

// Suggestion
func use(key: some EdDSAKey)
// Or, if one feels EdDSA is important to note
func use(edDSA key: some EdDSAKey)

Describe the solution you'd like I would like the new JWT API to not deviate from the current chosen API syntax by reinstating the use syntax function, instead of the add or add<keyAlgorithm>.

Additional context As a half postscript, the choice to have the function addEdDSA(key: some EdDSAKey) amongst others is, in my honest opinion, not conform API guidelines set out by Swift. I would also suggest the omission of the key: parameter name, i.e.: addEcDSA(_ key: some EdDSAKey). I feel this is as readable at call site if not more readable.

[ptoffy]

As for using add(edDSA:) instead of addEDSA, I think that's a sensible thought and I'll look into updating the package to use that. As for replacing add with use, IMHO add is a better fit as it makes a better idea of the keychain approach that the key collection uses, meaning there's different keys which can be added and used in different contexts. I'd expect use to replace the key that's there already. Does that make sense?

[Craz1k0ek]

Does that make sense?

Partially; I'm fine with the add syntax for a collection, that's common. However, the add also makes me wonder if there is a remove (spoiler alert: there isn't). Again, for the collection itself, not a big problem, however, for the configuration this is a bit of an odd experience.

Look at how the database works:

let psql = try SQLPostgresConfiguration(url: "postgres.com")
let sqlite = try SQLiteConfiguration(storage: .file(path: "myfilepath.db"))

application.databases.use(.postgres(configuration: psql), as: .psql)
application.databases.use(.sqlite(sqlite), as: .sqlite)

Technically, we're adding multiple databases/configurations to the application, yet, there is no add syntax here either. The application is able to use these databases/configurations. I would assume similar behaviour for JWT/JWK where we tell the application "Use these keys". The fact that one adds these keys in some storage is not relevant for the caller, nor is the fact that it's stored in some Keychain like structure.

Again, I thinks it's of upmost importance that we keep the API as uniform as possible, so that consumers know how to use new API's and functionality, without having to figure out new syntax. The add function does not contribute in that regards.

[0xTim]

I think the proposed change of

// Suggestion
func add(key: some EdDSAKey)
// Or, if one feels EdDSA is important to note
func add(edDSA key: some EdDSAKey)

Makes sense to fit better with the Swift API guidelines.

In terms of use vs add, I disagree. Whilst the package is part of Vapor, it's used outside the Vapor ecosystem and should be seen as the next steps for Vapor's API as opposed to fitting in with the old Vapor 4/Fluent APIs that may or may not make sense in a future version.