feat: Replace config.state with callback

[nrosenwald]

What does this pull request change?

This PR adds TAuthConfig.stateFn, a callback for generating a random state String. The appropriate documentation is also updated.

Why is this pull request needed?

This allows for applications to generate and use states without having to manually generate a state every time IAuthContext.logIn is called. This also enables states to be used in the initial log in when TAuthConfig.autoLogin is true.

Issues related to this change

None

[nrosenwald]

I've quickly realized that I shouldn't remove TAuthConfig.state. That would be an obviously breaking change. I'll update this tomorrow. I'll update the set state to be const state = customState ?? config.state ?? (config.stateFn && config.stateFn())

[sebastianvitterso]

Hey, @nrosenwald! Thanks for contributing!

At first glance, the code looks alright, but I'd like to know more about the use-case for it. Could you describe the scenario where this improves developer experience, with the context around the library as well?

Thanks!

[nrosenwald]

Hi @sebastianvitterso! Of course. There's two main reasons.

1. Auto login is very useful, but (as far as I can tell) the only way to set a state parameter for the auto login authorize request is to set a state in an app's TAuthConfig. Doing so also means that any proceeding authorize requests will use the exact same state. Typical best practice is to ensure that the state is different for every authorization procedure, which that isn't currently possible when TAuthConfig.autoLogin: true and there are multiple logins within the same browser session.
2. Currently, if developers are using the state?: string parameter in IAuthContext.logIn, they must call something like context.logIn(generateRandomState(), undefined, 'redirect') every time a login is required. With this PR, if a developer provides a state function (like TAuthConfig.stateFn: generateRandomState), then any login calls can be refactored to context.login(undefined, undefined, 'redirect'). This simplifies the calls and helps prevent bugs in case a developer were to forget to add a state to a login call.

[sebastianvitterso]

Cool! Thanks for the PR! @soofstad thoughts on this?

[soofstad]

Thanks for the PR @nrosenwald! Have had something like this in mind as well :) Just thinking, there should be no reason for the user to provide this function, if the library itself generated a random state string when no state were provided. Except that it could be a breaking change to always include the state parameter even if the lib user did not specify one (I know, it's part of the spec, but I have very little trust in many AuthProviders to actually follow the spec). So I think this is a good compromise before v2.0.

A few things:

• Would like to see a unit test on this
• Is generateStateCallback a better name than stateFn? We try to avoid abbreviations
• If you could add a deprecation warning and TODO for version 2.0 on this. Just so we are clear that it might change in the future

[sebastianvitterso]

How's progress going, @nrosenwald?

[github-actions[bot]]

Stale pull request message

[sebastianvitterso]

@nrosenwald Feel free to re-open/re-create this if you want it merged!