Sharing Similar Project

[acelinkio]

Wanted to reach out to make sure you were aware of https://github.com/allenporter/home-assistant-openid-auth-provider. Appreciate your efforts on everything and wanted to make sure you were aware of it!

Cheers

[christiaangoossens]

@acelinkio As many wise Youtubers have said before: "it only takes publishing something on the internet, to instantly be proven wrong". I was not aware of that project.

I support home-assistant/frontend#23204, which would also improve the flow of this project, but much more will need to be done over there as well to support all IdPs on mobile, as HA uses a really old webview. It's effectively an improved version of home-assistant/frontend#14471.

@allenporter Thank you for your contributions to the community in https://github.com/allenporter/home-assistant-openid-auth-provider. What a coincidence that two people started working on an issue that was dormant for 2 years ;).

If you want, we can combine the repositories and work on it together, it seems that we followed a similar approach to the actual OIDC client, I just wanted to make it work with all clients before your PR gets merged (if it doesn't get pushback). It should even be possible to offer both (redirect & code copy flow) at once in the same lib. If you're open to working together, let me know.

[christiaangoossens]

@allenporter I would like you as an additional contributor on this repository as well. I don't mind being the main maintainer here, but I don't mind hosting it anywhere else either. Up to you what you think is best, I know this repo has a lot of link traction already, even though it was dormant, but I'm also open to migrating to yours.

I would welcome the changes that 'you are ahead in' as PR's on this repo:

• Configuration through the UI (I postponed that because it might change a bit at the start)
• Knowledge about specific HA API use/Python code quality improvements/error handling
• Any dev containers/tips you can give me as a Python dev; I used the Home Assistant Docker container as a dev env + Rye to keep my sanity; and/or Github Actions for the linter & validation for HACS

Would you mind going through my proof of concept code and checking which things you can contribute from yours?

I have some of the others on my TODO list:

• Supporting the Client Secret flow instead of only PKCE/public for those IdPs requiring that
• Configuration of which fields are to be used from the id_token (from the UI possibly)
• Fallback to userinfo if id_token does not contain the values

I'm not sure if we should implement the "improved" UX here already before it's merged in frontend and tested on mobile. Maybe we can register two providers from one plugin with both flows using the OpenID Client class and some of the endpoints?

[allenporter]

Yeah, happy to help improve this project since it was first and has some traction.

The code in my project for the auth provider was primarily lifted from a core PR referenced in the readme. I added the parts for config flow, streamlining registration, etc.

I would welcome the changes that 'you are ahead in' as PR's on this repo:
Would you mind going through my proof of concept code and checking which things you can contribute from yours?

As far as the code base goes here are my hot take, unfiltered:

• I don't have an opinions on the openid specific logic and features supporte. My impression is that home-assistant-openid-auth-provider supports client id and secret given that was what was supported by the IDP someone told me how to test this with and appeared to be natively supported by home assistant OAuth libraries. My primary focus in home-assistant-openid-auth-provider was on custom component code quality conforming to home assistant best practices and existing components.
• The home-assistant-openid-auth-provider lets you decide which email or sub are allowed to match from the id token in the configuration flow now.
• The hass-oidc-auth provider appears to overwrite all other auth providers. That seems like a problem?
• Config flow and UI configuration. Don't use yaml at all. Using the UI allows for multiple entries, and any future extensible like repair issues, other entities, anything else needed.
• I'm not sure why the extra endpoints are needed, or why the database persistence is needed. Maybe it's because of different assumptions about having frontend support for external steps in the auth login flow. home-assistant-openid-auth-provider assumes that full authentication flow support can be used with small modifications.
• The separate oauth client library in hass-oidc-auth is nice, removing some logic out of the provider. home-assistant-openid-auth-provider assumes that the core LocalOAuth2Implementation can do most of the lifting, but also has some additional logic in the auth provider. Perhaps there is a middle ground of pulling out openid logic from the provider and into an oauth implementation subclass and treating it as a client library.
• Tests. There don't appear to be any in hass-oidc-auth so that could be improved. home-assistant-openid-auth-provider is 85%+ covered by tests.
• As for mobile support, i have not tested it at all, so can try that. My assumption was that using standard authentication flows would be the simplest way to start to get parity though.
• Not sure there is much to say about devcontainer support other than that home-assistant-openid-auth-provider has a .devcontainer. This is part of a template for custom components I use for all of my custom components managed with cruft for keeping them in sync.

Maybe we can register two providers from one plugin with both flows using the OpenID Client class and some of the endpoints?

Are both providers needed for different purposes or is this a short term suggestion? I'm not really planning to do any more extensions on home-assistant-openid-auth-provider until the frontend PR support is added since its required for to work as part of the design requirements.

[christiaangoossens]

@allenporter Thanks for your first impressions, very helpful. Much of the different approach (extra endpoints, db) is based on my initial tests 2 years ago, where the route home-assistant-openid-auth-provider takes did not work on mobile without a pretty significant Android and iOS patch. Interested to see if you can confirm that your package works on mobile now with the frontend patch applied (and no mobile patches).

Assuming that's not in the cards soon, I made this alternative code based flow, which works on both mobile & desktop (without any patches), as long as you are willing to have a different starting URL. Depending on what works and what doesn't, both may need to co-exist. I don't have options right now to load translation strings into the frontend, therefore I cannot add actual errors and a description to the code input. If you have any ideas on how to get that to work (preferably without a frontend patch, or a generic one), that would be very helpful.

Other quick notes:

• The different OIDC Client was to support PKCE, the recommended route for modern web apps/public clients. The HA one seems to only support a very specific use case and half of the code is spent on refresh tokens which we will likely never use for a web client. I have put "doing a deep dive on the differences and possibilities to use it as a parent class" on the TODO list.
• Config options will change later on, I am thinking about if we should support the UI flow, or just keep it on YAML. Ease of use (a major sticking point for the HA devs: they did not want this to be too easy to setup) is less important than stability and the ability to have it work again by just putting back the yaml from backup. Open to suggestions here, may create a discussion post about it.
• Tests are a TODO ofcourse, although the tests that exist in the original provider PR, are good for code quality, but not for the actual end-to-end process as far as I can see. I will add full E2E tests too.

[allenporter]

Very helpful context. I'm unlikely to want to converge on yaml and an untested component. That said, super happy to keep sharing learnings and/or help advertise this component as one with more oidc offerings.

[christiaangoossens]

Very helpful context. I'm unlikely to want to converge on yaml and an untested component. That said, super happy to keep sharing learnings and/or help advertise this component as one with more oidc offerings.

Keep in mind that although the idea existed for 2 years, this integration is only 2 days old. So lots of subject and OIDC knowledge here, but a total beginner at HA integrations. Hopefully this gets to a place where everyone is happy about it.

For the yaml, that's worth a discussion post with the community weighing in for me, there are positives and negatives for it. I don't want to make that decision alone, so let's discuss it first after the initial testing is done. Discussion post at: #6