docs: document Entra password login and prerequisites - #1612
docs: document Entra password login and prerequisites#1612nooreldeenmansour wants to merge 4 commits into
Conversation
949a084 to
e2f3301
Compare
66d756b to
d6fe1b2
Compare
54c61dd to
b585d10
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1612 +/- ##
==========================================
- Coverage 87.33% 84.99% -2.35%
==========================================
Files 124 25 -99
Lines 8907 1939 -6968
Branches 111 0 -111
==========================================
- Hits 7779 1648 -6131
+ Misses 1072 291 -781
+ Partials 56 0 -56 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
b585d10 to
0d09eaf
Compare
0d09eaf to
8fcfd51
Compare
8fcfd51 to
40db6c3
Compare
| a URL to visit in a browser to complete authentication. This is the standard | ||
| OIDC device authorization flow. |
There was a problem hiding this comment.
Here we could use the correct technical term:
| a URL to visit in a browser to complete authentication. This is the standard | |
| OIDC device authorization flow. | |
| a URL to visit in a browser to complete authentication. This is the standard | |
| OAuth 2.0 Device Authorization Grant flow. |
There was a problem hiding this comment.
However, I'm not sure if the sentence actually provides any value to the user. Let's wait for Shane's feedback.
There was a problem hiding this comment.
For a guide, we usually assume some baseline knowledge on the part of the user and don't want to get in their way. So, the question is: would the typical user need this information confirmed or clarified when doing their configuration?
Otherwise, I would favor using the technical term. It isn't many extra words.
There was a problem hiding this comment.
So, the question is: would the typical user need this information confirmed or clarified when doing their configuration?
I don't know. From the research we did for #1615, I assume that "device code flow" is common enough that everyone who knows what "OAuth 2.0 Device Authorization Grant" is also knows that "device code flow" (or "device flow") is used to refer to it.
If we want to mention the technical term somewhere, maybe the dedicated "Authentication flows" reference page that we have now would be a better place?
| Group membership is read from the Microsoft Graph API. The token obtained from | ||
| the **Entra password + MFA** flow is issued by the Microsoft Broker App and does | ||
| not carry the `GroupMember.Read.All` scope, so the groups are resolved in one of | ||
| two ways: |
There was a problem hiding this comment.
I don't think the user knows what the "Microsoft Broker App" is. We should try to rephrase this, but I'm not sure how. Let's maybe wait for Shane's feedback on this.
There was a problem hiding this comment.
Agreed, it might a bit specific, I looked at himmelblau's docs, searched for "Broker", and there are no mentions of it either.
There was a problem hiding this comment.
I think we could use some more generic terms; such as Microsoft's client | Microsoft's sign-in client | Microsoft auth client
or something that conveys the same meaning/phrasing
There was a problem hiding this comment.
This section does seem a little convoluted. What is its purpose, to help users configure groups? If so, is all of the explanatory material needed? Should some of this info go in the config guide? Is some of the information, such as the use of MS Graph, Broker APP, GroupMember.Read.All scope already mentioned in the config guide? Is the requirement for the client_secret under [oidc] mentioned in the config guide?
I would consider whether there is already sufficient information to help users do what they need to do in the config guide. If so, ask what is the purpose of this section (relevant to security? concise reference? deep explanation?). If it's for a technical explanation, then maybe we need a dedicated, standalone explanation for the components and flows involved.
There was a problem hiding this comment.
This section does seem a little convoluted. What is its purpose, to help users configure groups?
Groups resolution for the Entra flow has two specific requirements register_device being true, or providing an OIDC client secret (app-only) with the GroupMember.Read.All scope
Should some of this info go in the config guide? Is some of the information, such as the use of MS Graph, Broker APP, GroupMember.Read.All scope already mentioned in the config guide? Is the requirement for the client_secret under [oidc] mentioned in the config guide?
We can probably augment that section and add images (similar to application registration), similar to how it is done in the config guide for device auth flow, but maybe we can keep that in a separate PR, so that we can discuss that more clearly
This section's content can also be integerated with the other documentation sections, in case we wished to keep the groups-management.md, but I do think that extra section is likely helpful as a resource/reference
| Microsoft Entra ID supports the following authentication methods: | ||
|
|
||
| - **Device code authentication**: The user visits a URL and enters a code to | ||
| authenticate. This is the traditional flow and works with all account types. |
There was a problem hiding this comment.
Does the other flow not work with all account types?
There was a problem hiding this comment.
There are specific scenarios where the Entra Password + MFA flow would be un-usable, they all boil down to the MFA method not being available for use:
- New user that might not have configured MFA yet
- Conditional policy to enforce an unsupported MFA method (currently, passwordless/fido/passkeys MFA methods redirect to DAG)
For such scenarios, we fallback to DAG
There was a problem hiding this comment.
Nitpick: I would avoid the term "traditional" here? It is ambiguous.
Recommended? Default? Conventional? Common?
I would prefer if we just stated the facts. In this case: this flow works with all account types.
There was a problem hiding this comment.
Agreed, dropped the traditional word, and went with stating the fact directly
b757084 to
7aadb40
Compare
edibotopic
left a comment
edibotopic
left a comment
There was a problem hiding this comment.
Thanks for the additions. Looks good. I have a few comments for you to consider.
| Microsoft Entra ID supports the following authentication methods: | ||
|
|
||
| - **Device code authentication**: The user visits a URL and enters a code to | ||
| authenticate. This is the traditional flow and works with all account types. |
There was a problem hiding this comment.
Nitpick: I would avoid the term "traditional" here? It is ambiguous.
Recommended? Default? Conventional? Common?
I would prefer if we just stated the facts. In this case: this flow works with all account types.
| ```ini | ||
| [flows] | ||
| ## Enable direct Entra password + MFA authentication (default: true) | ||
| #entra_password = true |
There was a problem hiding this comment.
#entra_password = true
Why isn't this just #entra_password_and_mfa?
It is longer, but nobody has to type it.
There was a problem hiding this comment.
Good note, I think since MFA is enforced and expected at all times for the Entra flow for either password or passwordless auth, we might even consider changing it altogether, because the entra_password name itself might be a bit inaccurate now for passwordless methods. But that's something for #1723
I think we can do entra_mfa (dropping, password references altogether which has similar style to device_code)
There was a problem hiding this comment.
I think since MFA is enforced and expected at all times for the Entra flow for either password or passwordless auth
Technically, the passwordless auth can be a single factor authentication, for example requiring only access to the Authenticator app or the FIDO key, right?
I would still be fine with calling it MFA if we think that it's easier for users to understand. We could also consider entra_auth or entra_native_auth.
There was a problem hiding this comment.
If it's not necessarily multi-factor in all cases, and doesn't always require a password, then maybe entra_authd is the right option here, perhaps with an updated explanatory comment.
| Group membership is read from the Microsoft Graph API. The token obtained from | ||
| the **Entra password + MFA** flow is issued by the Microsoft Broker App and does | ||
| not carry the `GroupMember.Read.All` scope, so the groups are resolved in one of | ||
| two ways: |
There was a problem hiding this comment.
This section does seem a little convoluted. What is its purpose, to help users configure groups? If so, is all of the explanatory material needed? Should some of this info go in the config guide? Is some of the information, such as the use of MS Graph, Broker APP, GroupMember.Read.All scope already mentioned in the config guide? Is the requirement for the client_secret under [oidc] mentioned in the config guide?
I would consider whether there is already sufficient information to help users do what they need to do in the config guide. If so, ask what is the purpose of this section (relevant to security? concise reference? deep explanation?). If it's for a technical explanation, then maybe we need a dedicated, standalone explanation for the components and flows involved.
587cc96 to
273f71c
Compare
Since the `entra_password` flow is the default choice for Entra (when both flows are configured and available for use), it makes sense to re-order documentation and broker configuration where the entra password+mfa flow is listed first
Document the Entra password + MFA flow, the `[flows]` configuration, and the security model around password hashing and token-derived user info. Also explain when the flow falls back to device authentication and how group lookup works with device registration or a client secret.
273f71c to
044f112
Compare
edibotopic
left a comment
edibotopic
left a comment
There was a problem hiding this comment.
All major comments addressed. Approved.
| Microsoft Entra ID supports the following authentication flows: | ||
|
|
||
| - **Device code flow**: The user visits a URL and enters a code to authenticate. | ||
| - **Entra password + MFA**: The user authenticates with their Entra ID password, |
There was a problem hiding this comment.
A reminder to update this if we decide to rename the Entra auth flow, as discussed in #1612 (comment)
Split the 'Authentication flows' section out of reference/identity-providers.md into its own reference/authentication-flows.md page. This makes the flows content easier to find and allows each provider entry to stay focused. The identity-providers table now shows a 'Flows' column linking to the new page, replacing the 'Provider docs' column (provider links are preserved in a 'Provider documentation' section at the bottom of the page). The Entra password + MFA compatibility details (fallback conditions) are moved into a dedicated 'Compatibility and requirements' subsection, breaking the complex single sentence into an itemized list.
7277092 to
cdd589c
Compare
The 'entra_password' name was misleading: the flow is not limited to password authentication and MFA is not always multi-factor (e.g. passwordless via Authenticator app counts as single-factor). 'entra_auth' is the more neutral and accurate term, paralleling the style of 'device_code'. References updated in the broker configuration template and the how-to guide.
cdd589c to
f8d7e52
Compare
| ## entra_auth: When true (default), users can authenticate directly with | ||
| ## Microsoft Entra ID using their password or a supported passwordless method. | ||
| #entra_auth = true |
There was a problem hiding this comment.
Should we allow disabling the passwordless authentication here? Maybe with a allow_entra_passwordless setting?
There was a problem hiding this comment.
(if we decide we want that, the implementation should be in a separate PR)
There was a problem hiding this comment.
Should we allow disabling the passwordless authentication here? Maybe with a allow_entra_passwordless setting?
What would be the expected behavior here? to redirect to DAG or fail the authentication if the user has a passwordless MFA method and that setting is disabled?
or then the user would be required to enter a password at all times? (Not sure if this can contradict the Entra's policies/configuration, but I can try to experimenting with that)
Usually I don't like extending the config with too many flags, but I think it's something we can consider if the users requested it, I checked himmelblau conf, and they don't have an equivalent config. But let's test and validate the scenarios further before deciding. Maybe there is a valid use case for that.
Created https://warthogs.atlassian.net/browse/UDENG-10999 to track and investigate this further
There was a problem hiding this comment.
Usually I don't like extending the config with too many flags, but I think it's something we can consider if the users requested it
agreed, let's wait for users actually requesting such a feature before spending time on it
| When `entra_auth` is enabled, the user can enter their Entra ID password | ||
| directly. After password verification, Microsoft Entra ID requires a second | ||
| factor (MFA). The broker automatically handles the MFA challenge: | ||
|
|
||
| - **Push notification or number matching**: The user approves a notification on | ||
| their registered authenticator app. | ||
| - **TOTP code**: The user enters a time-based one-time password from their | ||
| authenticator app. | ||
|
|
||
| ```{admonition} FIDO2/WebAuthn security key support | ||
| :class: note | ||
| FIDO2/WebAuthn security keys are currently not supported for the Entra authentication flow. | ||
| If only FIDO methods are registered, the user is prompted to switch to the device code flow | ||
| instead. | ||
| ``` |
There was a problem hiding this comment.
At this point, it probably makes sense to also document the passwordless authentication here, and merge it after #1723 (which I expect to be merged soon anyway). WDYT?
There was a problem hiding this comment.
That's fine with me, we can do it in a separate commit in this PR. The only blocker would be waiting for https://gitlab.com/samba-team/libhimmelblau/-/merge_requests/165 to be merged
3 participants
Document the Entra password + MFA flow, the
[flows]configuration, and the security model around password hashing and token-derived user info.Also explain when the flow falls back to device authentication and how group lookup works with device registration or a client secret.
UDENG-10792