Add Domain Hint #363
Add Domain Hint #363
Conversation
| @@ -531,6 +532,33 @@ func WithLoginHint(username string) interface { | |||
| } | |||
| } | |||
|
|
|||
| // WithDomainHint pre-populates the login prompt with the IdP domain. | |||
| func WithDomainHint(domain string) interface { | |||
There was a problem hiding this comment.
Nit: Is it possible to refactor our code base so that we wouldn't need to duplicate this code snippet twice?
CC: @chlowell
There was a problem hiding this comment.
I refactored to avoid the code duplication
There was a problem hiding this comment.
Oh, thanks for consolidating the new WithDomainHint() and existing WithLoginHint() with a common underlying helper. My original comment was about the WET between confidential client and public client, though. All the implementation and their corresponding test case are currently duplicated in the two clients, while the behavior could ideally be shared among a common base of both confidential client and public client.
Not sure how much refactoring would be needed. I'll defer this topic to @chlowell .
There was a problem hiding this comment.
I think withHint() and its supporting type make the code more complex for no benefit--this approach doesn't lower the maintenance burden. It's better for WithLoginHint() and WithDomainHint() to simply set their respective fields directly.
Sharing one implementation of WithDomainHint() between confidential and public clients might be possible, but the constraints imposed by Go and the current implementation make it difficult and much more complex than parallel options in each package. I don't think it's worth pursuing.
Regarding a shared base for both client types, we do have one, but testing it isn't a substitute for testing the public API.
There was a problem hiding this comment.
Ok, I reverted the WithHint() code and now have separate WithLoginHint() and WithDomainHint()
eopeter
force-pushed
the
eoche/domainHint
branch
from
46a588b
to
eb42da4
Compare
Co-authored-by: Charles Lowell <10964656+chlowell@users.noreply.github.com>
|
Kudos, SonarCloud Quality Gate passed!
|
This PR Seeks to enable adding Domain Hint option to be used to accelerate the user to their federated IdP sign-in page. Or they can be used by a multi-tenant application to accelerate the user straight to the branded Azure AD sign-in page for their tenant.