Add OIDC/SSO authentication support (fixes #512)#1678
Add OIDC/SSO authentication support (fixes #512)#1678zach78954 wants to merge 3 commits intoTechnitiumSoftware:masterfrom
Conversation
|
Hey looks good but general question: |
|
Hey! Looks like I forgot to turn off my linter / auto formatter. I'll fix my branch to make the diffs more manageable. Thanks for looking at it! Please let me know if there's anything else y'all would like me to change or modify. EDIT: Fixed, diffs are now correct. |
Implements OpenID Connect (OIDC) Single Sign-On authentication to address issue TechnitiumSoftware#512. Features: - OIDC authentication via ASP.NET Core middleware - Support for multiple IdPs (Entra ID, Okta, Auth0, etc.) - Automatic user provisioning with configurable group mappings - HttpOnly cookie-based session management - Rate limiting for provisioning attempts - Comprehensive environment variable configuration - Docker secrets support for sensitive values - Security headers (CSP, HSTS, X-Frame-Options, etc.) - Backward compatible with existing local authentication Security: - JWT signature validation via OIDC discovery - Cryptographically secure cookie secrets (32-byte) - SameSite=Lax cookie protection - No secrets in frontend bundles - Proper error handling without information leakage Documentation: - Added SSO configuration to DockerEnvironmentVariables.md - Includes examples for major IdP providers - Environment variable reference with _FILE variants Closes TechnitiumSoftware#512
zach78954
force-pushed
the
feature/oidc-sso-support
branch
from
5526a1f to
90c13bd
Compare
DnsServerCore/www/js/auth.js
Outdated
| localStorage.removeItem("token"); | ||
| localStorage.removeItem("token_expires"); | ||
| // Explicitly delete the session cookie | ||
| document.cookie = "session_token=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/;"; |
There was a problem hiding this comment.
If the token is now managed via httponly cookie then this will fail.
There was a problem hiding this comment.
Thanks for catching that. I've refactored the auth flow to be fully compatible with HttpOnly cookies.
Changes pushed:
-
Logout Fallback: The Logout endpoint now checks the session_token cookie if the token query parameter is missing/empty, ensuring logout works without client-side token access.
-
Token Security: I stripped the token field from the api/user/login JSON response for standard sessions so it's no longer exposed to the client at all.
-
Client Cleanup: Updated auth.js to handle the missing token gracefully and removed the ineffective client-side cookie deletion code.
|
Thanks for the PR. Will review it in details. I just had a glance at changes and there are too many changes unrelated to the feature being implemented. This makes it really tough to accept the PR since it may be breaking existing code and adding new bugs so requires lot more efforts. For example, you have modified existing API without considering that it would break integrations that other people are depending on. |
|
Hello I agree, there are quite a few breaking changes here that could become problematic. That said, OIDC support is a feature many people would appreciate, so it would be great to find a way to add it. @ShreyasZare would you be open to taking a more incremental approach instead? For example, breaking this down into smaller focused changes such as:
also i think its a good idea to keep OIDC related code isolated (e.g. separate folder) so it doesn’t conflict with or rewrite existing auth flows Once that foundation is in place, the actual OIDC provider implementation could be added gradually and reviewed step by step. |
|
Thanks @ShreyasZare and @scinca for the review. I really appreciate the feedback. I completely agree that this PR grew to include a few possible breaking changes while implementing the necessary security hardening for OIDC. Given the scope, maybe targeting a v15.x or a .5 release seems much more appropriate than trying to merge this into a minor update. Regarding @scinca 's suggestion to break this down—I am definitely open to that approach if it makes the review process, and managing the changes easier. To provide context on the specific API and Auth changes:
I am happy to split this into smaller PRs or simply retarget this work for a future major release. Let me know what you prefer! |
|
It would be much better to have the PR to only include the changes required for OIDC implementation without breaking anything else. The HTTP API is not just for use with the web GUI but is a generic API that is being used with various other integration. So breaking how it works is not really a good idea. |
|
Sorry for the late reply. I’ve had some personal things to take care of. I will break this PR down so it only includes the necessary SSO changes to keep things clean. |
No issues. Will wait for the updated PR. |
Addresses feedback from PR review: - Restored MapGetAndPost for all endpoints to support existing GET integrations - Login API now returns tokens by default, preserving compatibility for generic clients - Added cookie_auth opt-in parameter for Web GUI to use secure HttpOnly cookies - Added #region SSO markers to isolate SSO-related code - Enhanced OIDC claim extraction with fallbacks for Azure AD/Entra ID - SSO users blocked from standard login form (must use OIDC flow) - Reduced HSTS lifetime from 1 year to 24 hours (homelab-friendly) - Added security warnings for client secret storage (code, docs, UI) - Added environment vars to configure rate limiting for SSO This commit ensures the SSO implementation adds new functionality without modifying existing API behavior.
|
Hi @ShreyasZare and @scinca, I have refactored the PR to address the feedback about unrelated changes, API compatibility, and security controls. The implementation is now fully backward compatible with no breaking changes (I think): API Compatibility
Security Enhancements (opt-in only)
Code Isolation & Quality
Configuration
Ready for review and thank you so much for your time and effort support this project!! |
|
@zach78954 thanks for the update. Will review this is detail soon. I would suggest that you remove HSTS since this was intentionally not implemented to avoid lockout issues. Like say, if the SSL cert renewal depends on the DNS server and someone updated zone config causing renewal to fail. In such case, the admin panel would fail to work due to SSL cert errors + HSTS. The DNS server does not have editable config and the HTTP API being the only option would make this into a serious issue. DNS server being part of core infra, not being able to access it in critical time is an issue. |
|
Hi @ShreyasZare, I have removed the HSTS code to ensure it aligns with the existing design and avoids any potential access issues. I'll hold the commit locally just in case there are any other changes or feedback you'd like me to address before I push the update. Thank you again for your time! |
|
@zach78954 Thanks. Will review all the changes soon and let you know. |
3 participants
Summary
This PR implements OpenID Connect (OIDC) Single Sign-On authentication for the DNS Server web console, addressing the feature request in #512.
Motivation
As discussed in #512, enterprise environments often require centralized authentication through identity providers like Microsoft Entra ID (Azure AD), Okta, or Auth0. This implementation enables:
Implementation Details
Architecture
Microsoft.AspNetCore.Authentication.OpenIdConnectmiddlewareKey Features
Authentication:
User Provisioning:
Security:
Configuration:
_FILEvariantsScreenshots
Login Screen with SSO Option:
SSO Configuration Panel:
User Management View:
Modified Files
Backend (C#):
IsSsoUserflagFrontend (JavaScript/HTML):
Documentation:
DockerEnvironmentVariables.md- Comprehensive SSO configuration guide with examplesTesting Performed
Breaking Changes
None. This is a purely additive feature. Existing deployments will continue to work without any configuration changes.
Related Issues
Closes #512