[keycloak-dev] Proposal: Improvements to IdpUsernamePasswordForm

[Dmitry Telegin]

Hello Marek,

I've implemented a standalone PoC and currently in the process of
preparing a contribution.

https://github.com/eth-cscs/keycloak-link

Some points to consider:

- seems like providers cannot ship their own drop-in message bundles,
similar to theme-resources/templates, so if a provider needs to
introduce new messages, it would demand the use of a custom theme. Do
you think that could/should be improved?

- there is no way for a provider to override existing template, like
e.g. using theme-resources/templates/login.ftl - it will always be
taken from a theme. I know this is probably by design, but could we
improve that too? Currently, too much is hardwired to login.ftl, so if
you intend to modify/augment it, you'll end up copy-pasting the code.
Could the hierarchy look like this? base theme [lowest priority] -> theme-resources [intermediate] -> custom theme [highest priority]

- LoginFormsProvider.USERNAME_EDIT_DISABLED form attribute is used
throughout the code for the following purposes:
1) disable editing the field proper;
2) hide "register" and "remember me" UI elements;
3) enable social provider filtering in LoginFormsUtil::filterIdentityProviders().

I'd suggest to split that into two, with USERNAME_EDIT_DISABLED controlling 
editability only, and some new attribute (like IDP_LINKING_MODE) responsible for the rest.

Please let me know what you think,
Dmitry

On Fri, 2019-04-05 at 08:07 +0200, Marek Posolda wrote:
> On 04/04/2019 23:59, Dmitry Telegin wrote:
> > Hi Marek,
> > 
> > On Thu, 2019-04-04 at 09:14 +0200, Marek Posolda wrote:
> > > Hi Dmitry,
> > > 
> > > On 04/04/2019 00:45, Dmitry Telegin wrote:
> > > > Hi Marek,
> > > > 
> > > > You absolutely right, UsernamePasswordForm does the trick. However, the login screen rendered by UsernamePasswordForm is different from that of IdpUsernamePasswordForm in the following aspects:
> > > > - IdpUsernamePasswordForm doesn't display the block with IdP/social buttons
> > > 
> > > You're right. Small addition: The IdpUsernamePasswordForm displays
> > > social buttons, but just of those identity providers, which are already
> > > linked to specified user. In other words, if you want to link your
> > > account to broker-A and your account is already linked to broker-B, then
> > > broker-B is displayed on the form. This way, you have possibility to
> > > re-authenticate not just with your password, but alternatively by login
> > > to already linked broker-B, which is already linked to your account and
> > > hence "trusted" to be used for prove your identity.
> > > 
> > > It seems that with your proposal in case that username is unknown, we
> > > won't display any brokers on the screen and hence it will be mandatory
> > > to do re-authentication by username+password?
> > 
> > Yes, that's correct.
> > 
> > > > - IdpUsernamePasswordForm renders the message relevant to IdP-linking-by-reauthentication, which is this:
> > > > 
> > > > federatedIdentityConfirmReauthenticateMessage=Authenticate as {0} to link your account with {1}
> > > > 
> > > > So, my requirement is to implement the appearance of IdpUsernamePasswordForm + behavior of UsernamePasswordForm. I think this could be done either by augmenting the former, or by merging the two authenticators into a unified one, that would exhibit different behavior depending on the context (normal login vs. reauthentication for IdP linking).
> > > 
> > > I suggest to update IdpUsernamePasswordForm authenticator. In case that
> > > EXISTING_USER_INFO is not there, we can do the behaviour like:
> > > 
> > > - User will need to provide both username+password. Hence username field
> > > will need to be enabled
> > > - Social buttons won't be displayed on the login screen
> > > - Message will be bit different. For example just: Authenticate to link
> > > your account with {1}
> > > 
> > > For the case when EXISTING_USER_INFO is available, I would like to keep
> > > the same behaviour as currently is.
> > > 
> > > WDYT?
> > 
> > This is exactly how I was planning to do it myself :) so if you greenlight this, I'll proceed with JIRA/PR.
> 
> +1
> 
> Marek
> 
> > 
> > Just FYI, I'm also planning to publish a "standalone" version of the authenticator to be used with Keycloak <= 5.0.0.
> > 
> > Dmitry
> > 
> > > Marek
> > > 
> > > > Please let me know which way seems better for you, with the idea in mind of having this contributed to upstream.
> > > > 
> > > > Thanks!
> > > > Dmitry
> > > > 
> > > > On Tue, 2019-04-02 at 15:21 +0200, Marek Posolda wrote:
> > > > > On 28/03/2019 17:06, Dmitry Telegin wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > I'm currently working to implement the following requirements:
> > > > > > - users are managed externally via LDAP, self-registrations disabled;
> > > > > > - there is an external IdP;
> > > > > > - generally, there is no way to automatically match IdP identity with Keycloak's one, so IdP linking will always be performed by the user manually;
> > > > > > - in order to do that, the user should click the IdP icon in the login screen, authenticate with the IdP, get back to Keycloak and "claim" his/her Keycloak account by entering correct username and password.
> > > > > > 
> > > > > > Currently, the closest thing in Keycloak is o.k.authentication.authenticators.broker.IdpUsernamePasswordForm (aka "idp-username-password-form", aka "Username Password Form for identity provider reauthentication").
> > > > > > However, it 1) prefills username field and makes it non-editable, 2) depends on the preceding IdpCreateUserIfUniqueAuthenticator execution to provide existing user model (EXISTING_USER_INFO auth note).
> > > > > > 
> > > > > > My proposal is to improve IdpUsernamePasswordForm by allowing its execution even without the preceding IdpCreateUserIfUniqueAuthenticator. In the absence of EXISTING_USER_INFO, IdpUsernamePasswordForm should allow the user to manually enter username.
> > > > > 
> > > > > I wonder if you can't already achieve something like this with the OOTB
> > > > > authenticator implementations, but just correctly configure them? For
> > > > > example in the "First Broker Login" flow used for your identity
> > > > > provider, you can just directly use the default browser-based
> > > > > authenticator ( UsernamePasswordForm ) instead of the
> > > > > IdpUsernamePasswordForm. That way, the username+password form will be
> > > > > always shown for "First Broker Login" and once user authenticates, his
> > > > > account will be linked with IdP account.
> > > > > 
> > > > > Marek
> > > > > 
> > > > > > Please let me know if you think it's worth having this in Keycloak. Regards,
> > > > > > Dmitry
> > > > > > 
> > > > > > _______________________________________________
> > > > > > keycloak-dev mailing list
> > > > > > keycloak-dev at lists.jboss.org
> > > > > > https://lists.jboss.org/mailman/listinfo/keycloak-dev
> 
>