Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable single sign-on (SSO) support natively #7649

Closed
jeremystretch opened this issue Oct 27, 2021 · 13 comments
Closed

Enable single sign-on (SSO) support natively #7649

jeremystretch opened this issue Oct 27, 2021 · 13 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@jeremystretch
Copy link
Member

NetBox version

v3.0.8

Feature type

New functionality

Proposed functionality

This FR seeks to introduce built-in support for single sign-on authentication using python-social-auth, and specifically social-app-django (formerly django-social-auth). This is a continuation of the discussion under #2328.

Currently, NetBox defines two authentication backends:

AUTHENTICATION_BACKENDS = [
    REMOTE_AUTH_BACKEND,  # 'netbox.authentication.RemoteUserBackend'
    'netbox.authentication.ObjectPermissionBackend',
]

This FR will allow administrators to either replace or supplement the default REMOTE_AUTH_BACKEND with one provided by python-social-auth. More detail on the proposed implementation can be found in the project's documentation.

I haven't dug into this much yet, so I'm not sure what sort of roadblocks we might run into. We'll also need to ensure to avoid any conflicts with NetBox's object-based permissions system, although those should be minor as that deals with authorization rather than authentication.

Use case

This will allow NetBox administrators to enable one or more of the supported authentication backends (e.g. Google, Okta, GitHub, etc.) for authenticating NetBox users, in addition to the local authentication and custom remote authentication options already provided.

Database changes

The Django app will install several models, but they will be managed outside of NetBox. I don't believe any database changes are necessary within NetBox itself.

External dependencies

social-auth-app-django and social-auth-core will be introduced as required dependencies. Additional packages may be needed depending on the configured backend(s), however they should be optional.

@jeremystretch jeremystretch added type: feature Introduction of new functionality to the application status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Oct 27, 2021
@nahun
Copy link

nahun commented Oct 27, 2021

This would be great to have built-in. I already patch in python-social-auth in my deployment like this: https://gist.github.com/nahun/5d4d715ca37a2465aaf59ab152413dc2

The main features I would look for:

  • Option on setting what attribute is used for the username. I use UPN (UserPrincipalName) today from AzureAD.
  • Ability to map roles to NetBox groups. It might need to lookup an arbitrary attribute name for the "roles".

No database changes are needed in my use case.

@jeremystretch
Copy link
Member Author

Awesome, thank you @nahun!

@yrro
Copy link

yrro commented Oct 28, 2021

A comment from the PoV of someone who will use this to authenticate against his organization's Azure Active Directory once the code lands:

Whatever OpenID Connect implementation you choose, please make sure that it uses the tuple of (iss, sub) claims to uniquely identify users, rather than any other claims such as email!

This is because user names change from time to time, and matching user records against the correct claim saves users a lot of time and effort when those renames happen.

Some docs for reference:

The sub (subject) and iss (issuer) Claims, used together, are the only Claims that [a Relying Party] can rely upon as a stable identifier for the End-User, since the sub Claim MUST be locally unique and never reassigned within the Issuer for a particular End-User, as described in Section 2. Therefore, the only guaranteed unique identifier for a given End-User is the combination of the iss Claim and the sub Claim.

All other Claims carry no such guarantees across different issuers in terms of stability over time or uniqueness across users, and Issuers are permitted to apply local restrictions and policies. For instance, an Issuer MAY re-use an email Claim Value across different End-Users at different points in time, and the claimed email address for a given End-User MAY change over time. Therefore, other Claims such as email, phone_number, and preferred_username and MUST NOT be used as unique identifiers for the End-User.

https://openid.net/specs/openid-connect-core-1_0.html#ClaimStability

and

When identifying a user (say, looking them up in a database, or deciding what permissions they have), it's critical to use information that will remain constant and unique across time. Legacy applications sometimes use fields like the email address, a phone number, or the UPN. All of these can change over time, and can also be reused over time . For example, when an employee changes their name, or an employee is given an email address that matches that of a previous, no longer present employee. Therefore, it is critical that your application not use human-readable data to identify a user - human readable generally means someone will read it, and want to change it. Instead, use the claims provided by the OIDC standard, or the extension claims provided by Microsoft - the sub and oid claims.

https://docs.microsoft.com/en-us/azure/active-directory/develop/id-tokens#using-claims-to-reliably-identify-a-user-subject-and-object-id

Anyway - it's not quite as simple as using sub for the username. Because then that horrible string that has no meaning to the user is displayed throught the user interface. For apps like NetBox that have an existing user model that expects a username, the right thing to do is use the preferred_username claim for the user-visible username, and add extra fields to the users table to store iss and sub (or munge them together into a single value if you don't want to rely on composite keys, though PostgreSQL supports them perfectly well AFAIK). These values are then only used for looking up a user record in the redirect handler in order to insert/update as appropriate.

Also - a +1 for updating groups based on the role claim during login. I can then offload all user role management to the IdP rather than doing it in NetBox itself. I am happy to try working on this if you want.

@jeremystretch
Copy link
Member Author

Well, given that the necessary modifications to core seem fairly minor, maybe it makes sense to include social-auth-app-django in the v3.1 beta. I suspect we'll need to do quite a lot of testing and make iterative improvements as we go. It might not be fully production-ready in the v3.1 release but it also shouldn't really interfere with anything.

@yrro We're going to be constrained by the backends provided by social-auth-app-django, which in your case seems like it would be AzureADOAuth2. I'm not familiar enough with this yet to say whether your concern will be addressed, but if not it should be fairly easy to write a custom backend that operates exactly as needed.

@jeremystretch jeremystretch self-assigned this Oct 29, 2021
@jeremystretch
Copy link
Member Author

Working off of the docs and @nahun's example I was able to get a bare minimum implementation of python-social-auth up and running pretty quickly in the 7649-social-auth branch. It's extremely bare bones at the moment, but should provide a sufficient test bed for additional work.

@nahun
Copy link

nahun commented Oct 31, 2021

Thanks Jeremy! I got my use case working in the 7649-social-auth branch as a test.

Here is what I put in configuration.py:

LOGIN_REQUIRED = True
REMOTE_AUTH_BACKEND = 'social_core.backends.azuread_tenant.AzureADTenantOAuth2'

SOCIAL_AUTH_AZUREAD_TENANT_OAUTH2_RESOURCE  = '{{ AzureAD App Registration Client ID }}'
SOCIAL_AUTH_AZUREAD_TENANT_OAUTH2_KEY       = '{{ AzureAD App Registration Client ID }}'
SOCIAL_AUTH_AZUREAD_TENANT_OAUTH2_SECRET    = '{{ AzureAD App Registration Client Secret }}'
SOCIAL_AUTH_AZUREAD_TENANT_OAUTH2_TENANT_ID = '{{ Azure Tenant ID }}'

SOCIAL_AUTH_PIPELINE = (
    'social_core.pipeline.social_auth.social_details',
    'social_core.pipeline.social_auth.social_uid',
    'social_core.pipeline.social_auth.auth_allowed',
    'social_core.pipeline.social_auth.social_user',
    'social_core.pipeline.user.get_username',
    'netbox.custom_pipeline.set_username',
    'social_core.pipeline.user.create_user',
    'social_core.pipeline.social_auth.associate_user',
    'social_core.pipeline.social_auth.load_extra_data',
    'social_core.pipeline.user.user_details',
    'netbox.custom_pipeline.set_role'
)

And then I still placed a custom_pipeline.py in the folder with the same contents of my previous example. I don't know whats best with the pipelines: have some built-in official way of supporting custom pipelines or leaving it up to the user like this.

My only request is to set a "default" Auth Backend so that it doesn't load the /login page, it would immediately redirect to that backend. My users never need to see the NetBox login page itself. You'll probably need to build some method of forcing a login page for a local account. I haven't needed that myself, but I could see that being a need for others.

@jeremystretch
Copy link
Member Author

I don't know whats best with the pipelines: have some built-in official way of supporting custom pipelines or leaving it up to the user like this.

IMO it's best to leave it open to the user, much like we do with the REMOTE_AUTH_BACKEND setting. You're free to specify whatever you'd like, so long as it's been made accessible within NetBox' environment. Of course, it would make sense to include some commonly used functions out of the box, I just don't know what those are yet.

My only request is to set a "default" Auth Backend so that it doesn't load the /login page, it would immediately redirect to that backend.

We could expose Django's LOGIN_URL setting. Would that work? Or would you need some greater degree of flexibility to allow the actual URL to be inferred from the configured auth backend?

@jeremystretch
Copy link
Member Author

I'm going to tag this for v3.1. We'll likely continue to improve the implementation but I think even the minimal implementation achieved thus far is valuable.

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Nov 1, 2021
@jeremystretch jeremystretch added this to the v3.1 milestone Nov 1, 2021
@nahun
Copy link

nahun commented Nov 1, 2021

Of course, it would make sense to include some commonly used functions out of the box, I just don't know what those are yet.

My guess is my example of setting username and mapping roles to groups would be the most basic and common. Really is a guess though.

We could expose Django's LOGIN_URL setting. Would that work? Or would you need some greater degree of flexibility to allow the actual URL to be inferred from the configured auth backend?

For me, exposing LOGIN_URL is sufficient. The more user friendly way would be to infer the URL from the backend. Like choosing a default backend so that it changes the LOGIN_URL itself.

jeremystretch added a commit that referenced this issue Nov 2, 2021
jeremystretch added a commit that referenced this issue Nov 2, 2021
@jeremystretch
Copy link
Member Author

I've gone ahead and merged the MVP implementation for this. I expect to continue work under the feature branch during the v3.1 beta period as we receive additional feedback.

@l1bra2013
Copy link

Just wanted to add some feedback about this. Although adding the social-auth app works fine for frontend authentication, it doesn't provide a good mechanism for API auth. There is a mechanism provided by https://github.com/RealmTeam/django-rest-framework-social-oauth2, although the project appears dead

It may be best to adjust the netbox configuration.py to be more flexible in allowing the user to simply specify what, if any, authentication override they may want for both the frontend and backend

For example, if using Mozilla Django OIDC, that plugin provides the ability to enable both frontend and API auth overrides, but the following files need changing:

  • settings.py
    • Add in all of the config options for OIDC (prefixed with OIDC_)
    • Add mozilla_django_oidc to INSTALLED_APPS
    • If using API auth - add mozilla drf auth and optionally remove netbox token auth
    • Change login URL and redirect URL
  • Add OIDC plugin specific URLs to urlpatterns in urls.py
  • Modify login.html to include an OIDC login button, optionally remove built-in user auth, and optionally modify other templates as needed to adjust the logout button

These changes work with the existing REMOTE_AUTH_ variables, by specifying a Django compatible auth class (in this case mozilla OIDC)

I think that in the spirit of the REMOTE_AUTH mechanism, it would be better to allow the user to determine what auth mechanism works best for their use case, but provide some sort of mechanism for the user to make the necessary changes, outlined above and pretty much the same in the recently merged PR. If netbox incorporated some sort of improvements to allow for these changes as more of an addon/override rather than a modification to the netbox code, that may make everyone happy.

Then examples could be provided for common mechanisms such as social-auth, mozilla-oidc, ...

@marcus-crane
Copy link
Contributor

marcus-crane commented Nov 9, 2021

I guess I'll chime in with my hat as the maintainer of the somewhat spaghetti but still functional netbox-plugin-azuread that my previous employer and a few others have been using to authenticate into Netbox via Azure Active Directory.

First off, I'm really excited to see this functionality getting first class support. I'll be looking forward to the day when I can deprecate the plugin fully and point towards Netbox proper.

As far as functionality goes, I'm no Azure AD expert at all so the main things I would echo are what nahun already mentioned about attributes and mapping roles. In addition to that, it makes sense to filter roles too since it's a bit redundant importing potentially thousands of user roles when only a few are used. That said, I'm assuming you'd be mirroring Azure AD groups into Django land.

When it comes to UX, the main fiddly bits are login and redirect/reply URIs. I don't think the reply URIs matter so much (plugins/azuread/complete/) in my case. For the login url, my plugin is often used in conjunction with nginx to just proxy pass over the top of /login/ but of course, that's redundant when you control the source repo :)

For this case, perhaps it makes sense to just surface n number of buttons in the login template that show up as users enable different auth backends instead of treating it too differently. I can see someone enabling a billion backends and then complaining when buttons slide off the page though haha.

Anyway, I haven't had a play with the beta just yet so I'll let you know if I have any feedback and I'm looking forward to seeing it progress!

@yrro
Copy link

yrro commented Nov 9, 2021

That said, I'm assuming you'd be mirroring Azure AD groups into Django land.

That depends on the token claims configured upon the Azure AD App Registration by the Azure AD Application Administrator.

The proper (dare I say "modern") way to do it is to define roles (e.g,, viewer, editor, admin, staff) on the App Registration, and then configure the mapping of users (or groups) to roles upon the corresponding Azure AD Enterprise Application (which is confusingly also often described as "the service principal" in various places).

As a result, the the app's ID token will include a roles claim with just the application-specific roles configured.

The other, legacy way of doing it is to configure the App Registration's ID Token to include a groups claim, and then the Application Administrator has to decide which Azure AD groups should be included in the groups claim. This has a number of shortcomings and basically only exists to aid in migrating legacy apps away from AD FS. These are described at https://docs.microsoft.com/en-us/azure/active-directory/hybrid/how-to-connect-fed-group-claims if you want the gory details.

TL;DR: define roles on the App Registration, determine who gets what roles on the Enterprise Application, and the roles claim will include the assigned roles.

On the application side, something has to consume the roles claim of course. A typical Django app might set the user's is_admin and is_staff fields based on the presence of admin and staff roles. Those two are easy. In NetBox's case, it might make sense to map other roles to the IDs of particular Permission objects, defined statically in the config file...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

5 participants