Move IdP configuration information to IdentityProvider.register()

[ekovac]

This aligns closer with how the information is used.

[npm1]

Hmm maybe? register is currently thought to be for IdPs that want to be used in certain RPs which accept IdPs of a certain kind. This seems very different from lightweight, where the IdP is setting information to be used when the RP specifically requests this IdP to be used.

[ekovac]

Sure, but I think semantically "register" implies up-front declaration of something, and so having this configuration information live in IdentityProvider.register() makes sense. It doesn't feel like navigator.login.setStatus() is nearly as close of a fit.

[bvandersloot-mozilla]

I think I prefer re-using setStatus, so that we keep a single function call and don't rely on a static function. Ideally anyone registering should be setstatusing, so why make them do both? Maybe an argument name could convey the semantics we want.

[samuelgoto]

IdentityProvider.register() currently takes as input a Config file by reference (through a configURL), it seems natural to me for it to take it by value too.

[johannhof]

I would also prefer setStatus, I think. It just makes sense to me for the login status API to have this functionality (pass in user metadata for the browser to use in UI) and we should preserve that IMO

[samuelgoto]

IdPs also set login status via HTTP headers because it is easier to do that via 302s than executing JS, so if we were to use Login Status, we'd need to fit all of these endpoints into HTTP headers too, I think.

[ekovac]

Another point I'd like to make in favor of putting the config information in IdentityProvider.register() is that this configuration info should almost certainly have different expiration behavior than the cached account information, possibly a longer lifetime.

[npm1]

It feels weird to have this in either. register is used for IdP registration API, and in particular I would expect to see UI show as a result of this method. setStatus is used for the Login Status API and it should be invoked when the user is being logged in or out of an IDP. Even if we reused one of these, the semantics feel substantially different. So I'm tempted to say it should be a separate method as I think that is clearer. Could it be IdentityProvider.setConfig or something like that?

[johannhof]

Ah sorry I think I misunderstood "IdP configuration" to be "everything that is passed into setStatus". If this is just for the apiConfig piece, I don't have as strong of an opinion anymore. Using register / setConfig SGTM.

[bvandersloot-mozilla]

IdPs also set login status via HTTP headers because it is easier to do that via 302s than executing JS, so if we were to use Login Status, we'd need to fit all of these endpoints into HTTP headers too, I think.

Or not! The JS calls for register/unregister don't have a header equivalent now, so we don't need to make it into the header for setStatus.

[samuelgoto]

Or not! The JS calls for register/unregister don't have a header equivalent now, so we don't need to make it into the header for setStatus.

But what would we tell IdPs that are using HTTP headers now if they wanted to use lightweight?

Could it be IdentityProvider.setConfig or something like that?

FWIW, setConfig() could work.

[bvandersloot-mozilla]

We can have the semantics of the JS and HTTP interact such that the setstatus headers update the latest value only for the value for which it has the ability to update, leaving the other values (e.g. config info and hints) only modified by the Javascript