feat: adds config file and AuthTokenStorage

[fenilli]

This PR extends the current config to a file instead, by a virtual file via addPluginTemplate we can import it, and set the options of it with functions for authStorage as #43.

While it does do it, there are considerations and changes I would like to ask from the maintainer @manchenkoff, as one doing it like this will break backwards compatibility, so we could instead have both ways, instead where one is a smaller subset of the other ( module doesn't accept functions, so no auth storage ). And secondly, I would probably use it to change other places, as to not use CSRF when using auth storages, maybe changing the login/logout paths and so on, would like thoughts on it.

[fenilli]

1. Needs to fix the importing the helper function for type safety ( can't be in modules.ts ).

[jjjrmy]

a few questions for @manchenkoff ...

Do we want to have the ability to store the CSRF token using the same method?
Basically useCookie() would be the default implementation of the AuthStorage.
Then would we have a config variable (mode) to select how we are sending the the "token" to the server?
Either as set-cookie header or Authorization header.

[manchenkoff]

Closes #33

[manchenkoff]

Hey @fenilli, thanks a lot for the contribution! I will take a look in a couple of days and give feedback from my side. And also I'll check your questions @jjjrmy 👍

[jjjrmy]

or we can have a toggle mode for Cookie vs Auth Token, and use the default callback as localStorage for authToken unless overwritten within the config.

[manchenkoff]

a few questions for @manchenkoff ...

Do we want to have the ability to store the CSRF token using the same method? Basically useCookie() would be the default implementation of the AuthStorage. Then would we have a config variable (mode) to select how we are sending the the "token" to the server? Either as set-cookie header or Authorization header.

Yes, ideally I would expect different implementations for different scenarios, so we can change the mode to one available from enum or something. In the case of Laravel Sanctum, we would have 2 options: cookie and token, based on the selected mode we should change the whole behavior of the plugin and not conditionally combine them.

[manchenkoff]

or we can have a toggle mode for Cookie vs Auth Token, and use the default callback as localStorage for authToken unless overwritten within the config.

this is very similar to the approach you mentioned, but the problem with localStorage is that it does not work with SSR, and the whole point of the library evaporates 😄

[manchenkoff]

This PR extends the current config to a file instead, by a virtual file via addPluginTemplate we can import it, and set the options of it with functions for authStorage as #43.

I've already asked about more details on that in the review, but is it the only way we can support callbacks or complex types in the configuration?

While it does do it, there are considerations and changes I would like to ask from the maintainer @manchenkoff, as one doing it like this will break backwards compatibility, so we could instead have both ways, instead where one is a smaller subset of the other ( module doesn't accept functions, so no auth storage ).

Sorry, this one is very confusing for me 😄 so which option do you suggest w/o breaking backward compatibility?

And secondly, I would probably use it to change other places, as to not use CSRF when using auth storages, maybe changing the login/logout paths and so on, would like thoughts on it.

Agree, also mentioned it in one of the previous comments. Since the module configuration has a set of common values, I would assume that you can reuse all things like login/logout endpoints and redirects with both modes - cookie and token. The only difference I see is how CSR/SSR handles the request, so behavior should be changed according to the selected mode in the config. For instance, when we have mode: cookie we still request CSRF and pass it as a header, but w/o token checks and the opposite for second mode: token - we pass the token header w/o trying to enrich the request with CSRF-related data.

[manchenkoff]

I think it would be a bit overwhelming to have both bearer token and later csrf (in buildServerHeaders) since we know that only one strategy should be applied. By the way, I am unsure that it will work for both CSR/SSR, have you checked with some implementation of the storage?

[manchenkoff]

now I'm curious if it's even possible to use token storage with SSR? 😄 I've never used it that way

[jjjrmy]

I’ll have to look into that. but I think with a mobile app there is no SSR? I haven’t been able to test this with SSR because of previous issues. But I will research this, I think there are some things like asyncData for sharing data between Serve and Client

[manchenkoff]

I'd appreciate that, thanks!

[fenilli]

I've already asked about more details on that in the review, but is it the only way we can support callbacks or complex types in the configuration?

Yes sadly it is a limitation of Nuxt config file, because they serialize it before sending to plugins, as stated here #2822.

Sorry, this one is very confusing for me 😄 so which option do you suggest w/o breaking backward compatibility?

Yeh I should have made a better explanation there, but basically because we can't pass anything that is not serializable via nuxt.config we can't have functions there like what we would need to implement an interface for token saving, so for having backwards compatibility, I can see two scenarios.

1. We would just add the configFile property to the module one, and merge both the options from nuxt.config and the config file options if passed.
2. We could have a discriminating union, where you can pass to nuxt.config the module options, or the configFile, for a possible more complete options.

Agree, also mentioned it in one of the previous comments. Since the module configuration has a set of common values, I would assume that you can reuse all things like login/logout endpoints and redirects with both modes - cookie and token. The only difference I see is how CSR/SSR handles the request, so behavior should be changed according to the selected mode in the config. For instance, when we have mode: cookie we still request CSRF and pass it as a header, but w/o token checks and the opposite for second mode: token - we pass the token header w/o trying to enrich the request with CSRF-related data.

Agree with this point, not sure about the mode tho, one thing we could do is make the storage instead of being a storage implementation, but a HTTP request transformer, you would internally still be able to make storage, but also change the HTTP request when using get to pass csrf or auth headers depending on what you're using there, and we would have a default implementation of this using CSRF.

What are your thoughts on these?

[manchenkoff]

Yes sadly it is a limitation of Nuxt config file, because they serialize it before sending to plugins, as stated here #2822.

Got it, looks good to me!

Yeh I should have made a better explanation there, but basically because we can't pass anything that is not serializable via nuxt.config we can't have functions there like what we would need to implement an interface for token saving, so for having backwards compatibility, I can see two scenarios.

1. We would just add the configFile property to the module one, and merge both the options from nuxt.config and the config file options if passed.
2. We could have a discriminating union, where you can pass to nuxt.config the module options, or the configFile, for a possible more complete options.

I think I see the idea, so for me, as a user of this package I would say that the crucial part is the simplicity of the configuration and easy integration - what if we add tokenStorage only as a separate config file? In this case, everybody who uses the existing module with cookie-based authentication won't need to change anything, but for those interested in the token storage it will be enough to create a new file and define the implementations. This will allow us to merge the functionality w/o breaking compatibility and we'll be able to refactor/simplify it later.

Agree with this point, not sure about the mode tho, one thing we could do is make the storage instead of being a storage implementation, but a HTTP request transformer, you would internally still be able to make storage, but also change the HTTP request when using get to pass csrf or auth headers depending on what you're using there, and we would have a default implementation of this using CSRF.

Yeah, it is not necessary to have mode there, it was just an example for the concept. This way with transformers also sounds good.

To sum up, my main point is to keep this module as simple as possible for the end user and keep backward compatibility as much as possible for now

[manchenkoff]

is there any difference from the functional perspective?

[fenilli]

To be honest I'm not sure if there are differences, I had some problems with the return when inside the template ( it might been another problem that I fixed and didn't make any difference that syntax ), so I changed so both use the same syntax, I can change it back and see if both work as functional instead.

[manchenkoff]

The only concern I have that it might be different order of execution during plugin registration since we use nuxtApp directly instead of returning plugin to be registered via provide, it would be great if you can double-check that it works with the previous version as well

misclick

[fenilli]

I think I see the idea, so for me, as a user of this package I would say that the crucial part is the simplicity of the configuration and easy integration - what if we add tokenStorage only as a separate config file? In this case, everybody who uses the existing module with cookie-based authentication won't need to change anything, but for those interested in the token storage it will be enough to create a new file and define the implementations. This will allow us to merge the functionality w/o breaking compatibility and we'll be able to refactor/simplify it later.

I added back the options runtimeConfig and sanctum to the module, and made defu merge later with the config file. so now it should be backwards compatible with the addition of config files for complex configuration.

Yeah, it is not necessary to have mode there, it was just an example for the concept. This way with transformers also sounds good.

To sum up, my main point is to keep this module as simple as possible for the end user and keep backward compatibility as much as possible for now

Still need to think a bit on what should we do now to make it simpler for those "modes" to happen based on what, I think a transformer might be good, just need to think how would it transform and where it should do it, both for request and response, as we might need information from the response to be passed somewhere, so it can be saved ( for the token case ).

I was probably thinking in changing the AuthStorage idea to something more in line with changing the request/response headers. as the storage in itself is not necessary, it can be internal to the users project, he would just need to get the response from login to do anything he wants, and change the request headers, to send anything he needs for its implementation.

And we would use the cookies csrf as base if nothing is passed to it.

[fenilli]

Also do you know how to export other functions from a Nuxt module? we can't really export from the main entry point, nuxt doesn't like that, and a helper function like defineSanctumConfig would be very helpful for type safety in the config file.

[manchenkoff]

Also do you know how to export other functions from a Nuxt module? we can't really export from the main entry point, nuxt doesn't like that, and a helper function like defineSanctumConfig would be very helpful for type safety in the config file.

I don't but I looked it up in the repo you shared previously and it seems that they exported the utility function in the main file here. If it doesn't work, I would expect just something like Partial<SanctumConfig> in the config file

[fenilli]

I don't but I looked it up in the repo you shared previously and it seems that they exported the utility function in the main file here. If it doesn't work, I would expect just something like Partial<SanctumConfig> in the config file

they export at the main of the ui package, not the modules nuxt one, that is why it works, for some reason nuxt doesn't like when you export anything else in their module main export.

[fenilli]

Ok after thinking about a lot, there is one thing I might like and want opnions about @jjjrmy and @manchenkoff.

As of now this package is meant for basically plug and play default behaviour, and its really good for that, with that in mind, I like the simple JSON options that it has, but its meant only for cookies (csrf), but laravel sanctum also accepts token based calls, so it might be a good thing to accept also by default a simple token based approach.

What I was thinking on how to do it, as simple as possible and to keep backwards compatible, was to do a discriminated union for configurations, and let the user choose in the config via a manager if they want from cookie vs token, it will also enable to keep the config exactly the same but with one additional thing that users may tap into it, the storage and transformer.

For cookies is simple, just add storage as config for only the config file, use the base cookie, and if someone wants he can instead pass a function that follows the storage interface ( add, remove, get ) all the configuration would stay the same outside of those two new properties.

For tokens, we would make changes to the current code to use each manager correctly, and each manager would do its job to add/remove things from the headers, and token would have two main default strategies in the storage, local and cookies, but it can also pass a storage itself for mobile for example.

The transfomer would be useful for users to at the end of requests change the request options if needed for any reason they might think, making this package really simple if needed, but also accept complex instructions if needed.

[manchenkoff]

Hey @fenilli

What I was thinking on how to do it, as simple as possible and to keep backwards compatible, was to do a discriminated union for configurations, and let the user choose in the config via a manager if they want from cookie vs token, it will also enable to keep the config exactly the same but with one additional thing that users may tap into it, the storage and transformer.

Your suggestion sounds good, I don't mind having it like that. The only concern I have is the compatibility with the current fetch implementation since there is no way to modify onRequest and onResponse interceptors after instantiation, please keep that in mind when designing custom transformers functionality.

Besides that, we are good to go! Feel free to push changes when you have time to discuss further! And thanks a lot for your participation, I appreciate that! 😎

[manchenkoff]

Closing for now, we can re-open this one once we have updates or a new one

[fenilli]

Sorry fo the delay, just having a lot of things to do, but I will try to make a new one this week, also thinking if we should have a way to have one or another, or possibility to have both at the same time.