0.7.0 alpha Draft - Addresses ToDo(s), Security Improvements, & OP Interoperability

[btbutts]

Here's a detailed list of improvements I've been working on in my fork of 0.7.0. I am still working on some ruff cleanup todo's as there too many lines in oidc_client.py, some lines are too long, and other things it won't correc itself, and some other minor changes I need to make.

The following should give you an idea of what I've worked on since I saw for several of the features you're working on for the 0.7.0 release, you'd marked them as help wanted.

custom_components/auth_oidc/__init__.py

custom_components/auth_oidc/config/const.py

custom_components/auth_oidc/config/schema.py

Added Imports for New Configuration Constants: Included VERBOSE_DEBUG_MODE and NETWORK_USERINFO_FALLBACK from the configuration module.

• Purpose and Benefits:
  - VERBOSE_DEBUG_MODE (boolean default False) const is set via enable_verbose_debug_mode in HA configuration.yaml. The purpose is to provide a very intential switch users may enable to not only increase log verbosity, but also capture OIDC auth flows that would otherwise be discarded, but are significatnly helpful for troubleshooting failed RP > OP integration configs. Whilst it is possible to use browser-based capturing extensions, these had some limitations in my experience, and also don't decode all relevant info in the flows. Secondly, whilst it is possible to set the default log level for the integration via HA, this would pose a security risk if inadervtently left enabled. Providing a specific swtich makes capturing this info this very intentional. This switch will log content that would otherwise be discarded.
  - Reference: Relates to OIDC Core §3.1.2.1 (Authentication Request) for scope handling and general client robustness, though primarily a best practice for implementation diagnostics.
  
  - NETWORK_USERINFO_FALLBACK (boolean default False) is set via network.userinfo_fallback in HA configuration.yaml. Not all OPs provide a userinfo endpoint in their discovery document See: OIDC Discovery §3 (Provider Configuration Response), thus necessitating RP resiliency via robust client behavior. If set True, the userinfo endpoint is constructed from the issuer endpoint via <Issuer_FQDN>/userinfo.
  - Reference: OIDC Discovery §2 (OpenID Provider Metadata) notes that certain fields like userinfo_endpoint are RECOMMENDED but not REQUIRED, justifying fallbacks.
  

Expanded Required Scopes: Updated REQUIRED_SCOPES to include email in addition to openid and profile.

• Purpose and Benefits:
  - Ensures retrieval of email claims by default, enhancing user identification and claim processing without requiring custom configurations. Though email is not a required scope, it is standard among OPs. In fact, many don't even include the username or preferred_username claims, given that those OPs use emails as their own internal username reference. For this, they are not governed by OIDC spec for what they do outside of the OIDC protocol, or even necessarily what values they pass to a given claim. That said, given HA doesn't use emails, in fact the Nabu Casa team discourages it, I've added some processing to strip the "@" and everything following it whenever configuration.yaml's claims.username is set to email or e-mail, thus allowing flexibility with more OPs, whilst also keeping with HA norms (even without HA currently supporting extenral AuthProviders).
  - Passed New Parameters to OIDCClient Initialization

No breaking changes; the file maintains compatibility while extending functionality.

custom_components/auth_oidc/stores/code_store.py

• Enhanced Code Generation: Replaced random.choices(string.digits, k=6) with secrets.token_urlsafe(16).
  • Purpose and Benefits: Generates a longer, more entropic, URL-safe token, reducing risks of brute-force attacks or collisions in authentication handoffs.
  • Reference: While not directly specified in OIDC, this aligns with RFC 7636 (Proof Key for Code Exchange by OAuth Public Clients) §4.1 (Client Creates a Code Verifier), which recommends high-entropy verifiers (at least 43 characters) for security in code-based flows. The use of secrets module follows Python's secure random generation best practices.

No other changes; the modification is isolated and non-breaking.

custom_components/auth_oidc/tools/helpers.py

• New Function: compute_allowed_signing_algs: Computes allowed ID token signing algorithms from configuration and discovery document, defaulting to RS256 if unspecified, with warnings for mismatches.

  • Purpose and Benefits: Provides flexibility in algorithm handling, improving compatibility while maintaining security through defaults and logging.
  • Reference: OIDC Core §3.1.3.7 (ID Token Validation) requires clients to validate the alg header matches expected values. OIDC Discovery §2 specifies id_token_signing_alg_values_supported for provider-advertised algorithms, justifying computation and fallback logic.

• New Function: capture_auth_flows: Logs verbose debug messages and optionally captures request/response content to files for analysis, conditional on verbose mode.

  • Purpose and Benefits: Facilitates detailed tracing of OIDC interactions, invaluable for diagnosing issues without performance overhead in production.
  • Reference: No direct specification, but supports OIDC Core §16 (Security Considerations) for logging and auditing in secure implementations. File captures aid compliance with debugging requirements in RFC 6749 §5.1 (Successful Response).

Existing functions (get_url and get_view) remain unchanged.

custom_components/auth_oidc/tools/oidc_client.py

• Added Issuer Normalization and Mismatch Check in Discovery Validation: Normalizes and strictly compares the issuer from the discovery document against the expected value to prevent mismatch attacks.

  • Purpose and Benefits: Strengthens security by enforcing exact issuer matching, mitigating man-in-the-middle risks or misconfigurations.
  • Reference: OIDC Core §3.1.3.7 (ID Token Validation) mandates checking the iss claim matches the expected issuer. OIDC Discovery §3.1 (Authorization Server Metadata Validation) and RFC 8414 §3.1.2 (Issuer Identifier) emphasize exact matching of the issuer (scheme/host only, case-insensitive scheme).

• Added Verbose Debugging Integration: Incorporated verbose_debug_mode and capture_dir for logging and file-based capture of flows (e.g., in fetch methods), initialized if enabled.

  • Purpose and Benefits: Enables comprehensive tracing of OIDC requests/responses, aiding diagnostics while optional to avoid overhead.
  • Reference: Aligns with OIDC Core §16 (Security Considerations) for monitoring and auditing authentication flows.

• Flexible Signing Algorithm Handling: Updated ID token validation to use computed allowed algorithms, warning on unsupported ones instead of failing, with fallbacks.

  • Purpose and Benefits: Increases robustness for diverse providers, reducing setup errors while prioritizing secure defaults like RS256.
  • Reference: OIDC Core §3.1.3.7 (ID Token Validation) requires verifying the signature using advertised algorithms from id_token_signing_alg_values_supported (OIDC Discovery §2).

• Added Caching for Discovery Document: Implemented instance-level caching with a 1-hour TTL to minimize repeated fetches.

  • Purpose and Benefits: Optimizes performance and handles endpoint rotations without staleness.
  • Reference: OIDC Discovery §4 (Obtaining OpenID Provider Configuration Information) allows caching but recommends periodic refresh; the TTL balances efficiency per best practices.

• Userinfo Endpoint Fallback: If userinfo_endpoint is absent in discovery, constructs it as issuer + "/userinfo" when fallback is enabled.

  • Purpose and Benefits: Ensures functionality with providers omitting this endpoint, enhancing compatibility.
  • Reference: OIDC Discovery §2 lists userinfo_endpoint as RECOMMENDED (not REQUIRED), permitting clients to derive it from the issuer per implementation conventions.

• Enhanced Exceptions and Logging: Added OIDCIdTokenInvalid and detailed logging/captures in methods like _fetch_discovery_document and _fetch_jwks.

  • Purpose and Benefits: Improves error granularity and traceability.
  • Reference: Supports RFC 6749 §5.2 (Error Response) for client-side handling.

These changes focus on security, performance, and compatibility whilst avoiding breaking alterations.

[christiaangoossens]

Hi @btbutts, thank you for your improvements. I will probably take some time to review the code as I don't have that much time for open source right now, sorry for that in advance. I'm prioritizing breaking bugs and security fixes.

However, I don't like the inclusion of the email scope by default, or the changes made to the email claim if it has been selected as a username. Relying on the client providing email in the scopes field if they do not use it anywhere is bad behavior from the IdP in my interpretation of the OIDC spec, as the scope is optional for both clients and servers. OIDC doesn't require the client to specify any scope, except openid itself to indicate that it would like an id_token, and IdP's should implement it as such. In this case, we also like profile as it's a well-known scope and it is actually used as specified in the OIDC spec in this integration. Users might also not expect the changes to username. Username linking is already a sensitive (and unsafe) subject, but making it more unpredictable for the end-user by doing transformations on email addresses doesn't sit well with me.

More importantly, supporting email (and theoretically profile) is also optional for IdPs and thus IdPs may give an error if email is requested if that scope is not assigned to the application. Preventing such errors, especially when we don't use the claim itself, is more important to me than compatibility with IdPs that require email to be present.

I am fine with adding better instructions for the email scope in the README, just like we do for many other common configs. Is that solution acceptable for you?

Furthermore, I don't think supporting userinfo that way is the best solution, as it's a very specific guess (other IdP's might have other paths). I am more in favor of allowing the user to override any endpoint (auth, token, userinfo etc) using the configuration, as suggested by #110.

For your IdP from #173, this might then require passing the additional scope email and setting the userinfo path explicitly, but I do still think that your IdP implementation would really benefit from not requiring that scope and to adding the userinfo_endpoint to the discovery.

Again thank you for your contributions and time debugging! Hopefully we can find a solution we're both happy with!

[btbutts]

Hi @christiaangoossens . Thanks for your feedback. It's appreciated. Understood regarding the email scope. That's easy enough to move it out of the default scopes and just include a config template for IdP's like OneLogin. I see that's something that you already started.

My mods are no where complete yet (there's a few things I've found I need to work out and complete some more testing), which is why I haven't submitted the PR for review yet. I really just wanted to get it on the radar for feedback whilst I work through it all (perhaps on what you all might value more).

For your userinfo comment, I think that makes perfect sense, rather than writing a static fallback endpoint into the project, it might make sense just to make that an option users can add in themselves, if needed. IdP's are not required by spec to provide the userinfo endpoint in the discovery doc; They are, however, required to support the userinfo endpoint. These IdP's will provide a static link which can be manually added to the user/org's RP config. Thus, this is why I added the fallback option. I actually don't need this for my own IdP. Both OneLogin and CSE already have it in their discovery documents so its not something I need myself for my own implementation. However, I did notice it, and figured its not too difficult since it's something I've come accross a lot at my day job. 😃 We're doing a lot in the SaaS space these days (ZTNA, SASE, and SSE).

The rest primary targets some ToDo's I found throught the project in the source, some of which I would benefit from, so I started building them out. I had already completed a working implementation of a lot of this on the 0.6.3 release, but figured since 0.7.0 seems to be the active branch, it makes sense to port a lot of it over to 0.7.0 before I continue further.

Troubleshooting capability logging was one deficit I found. The hardest SSO integrations, I find, are the ones that don't have an option for detailed logs. Whilst there are logs and specific exceptions already implemented throughout the codebase, some of that would be more helpful for debugging than troubleshooting a failing implementation. This is why I added an option for the verbose_debug_mode. There is value in having a method to capture the detailed requests and responses on both the RP and OP side of things. I was only able to get so far with a browser extension that captures OAuth and OIDC flows as that assumes the flow actually traverses the endpoint I'm capturing from. Most RPs and OPs have a way to capture this; some hide it behind thier support systems but given HA is a open source project, I figured it makes sense to give this to the user, but it should be something they explicitly want to enable, as well as limited in scope, for several reasons... For just general verbose logging, that should be done via HA's own logging config, E.G.: like an automation that sets the integration's log level. That said, the integration did not already have a way to capture this details flows info, thus I added an option for it.

The rest of the ToDo's really focused around certain areas of cryptography and randomization. I don't have time to go into more detail about that right now but basically, you already had a well working implementation. I really just saw some ways it could be strengthened for those IdP's that support it, yet should still function identically to how the integration did for those that don't.

Anyways. Thanks again for your response. It's well received. Cheers!