Minor broker refactoring and cleanup #349
Conversation
There was a problem hiding this comment.
Makes sense on moving the provider specific to non provider for now.
One question about firstSelectedMode. The idea was IIRC:
- you have an authentication, you select one auth method
- then, broker decides it’s MFA (which is not the current QR code method), and either ask for another auth method or is password definition/change (which on the local password definition is supported)
-> The goal was that next time you select the same user, the first auth method (if available) is auto-selected. Does this still work or does it work because by chance, we don’t set in the last local password reset the second time you log in?
AFAICT there is no functionality implemented for that. The PAM module currently autoselects the first element in the list of authentication modes returned by the broker in the |
The firstSelectedMode was set but never used.
The decision which authentication modes are offered is (currently) not provider-specific. Lets make the interface simpler until we actually have a need to make it provider-specific.
adombeck
force-pushed
the
broker-refactorings
branch
from
3897095 to
ac0a46a
Compare
| @@ -277,6 +273,33 @@ func (b *Broker) GetAuthenticationModes(sessionID string, supportedUILayouts []m | |||
| return authModes, nil | |||
| } | |||
|
|
|||
| func (b *Broker) availableAuthModes(session session, tokenExists bool, endpoints map[string]struct{}) (availableModes []string, err error) { | |||
There was a problem hiding this comment.
IIRC, the purpose of this being considered provider-specific was to allow them more control over what is or isn't supported (i.e. EntraID can have modes that Google doesn't and so on...). It also makes sense to have it work as:
"general broker API: ok, this is what I have and what I can do, how will we do this?
provider API: Ok, so let's go like this..."
I know this is not something we interact with currently (and it results in a horrific function signature), but is it worth refactoring this now to maybe have to redo this later? We know that authd can handle TOTPs, codes and so on, so it could be something that we allow in the future...
Remove an unused field and move a function that's not provider-specific from the provider interface to the broker. See commit messages for details.