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

Support automatic user creation with SAML authentication #3551

Merged

Conversation

ZauberNerd
Copy link
Contributor

@ZauberNerd ZauberNerd commented Jun 11, 2024

With this change we are able to automatically create users for SAML authenticated users and assign them roles.
Before this change, users had to be created manually on the velociraptor server, even when using SAML authentication, which lead to double book keeping.

Based on the work in the certs authenticator module:

  • users_manager := services.GetUserManager()
    user_record, err := users_manager.GetUser(r.Context(), username, username)
    if err != nil {
    if errors.Is(err, utils.NotFoundError) ||
    len(self.default_roles) == 0 {
    http.Error(w,
    fmt.Sprintf("authorization failed for %v: %v", username, err),
    http.StatusUnauthorized)
    return
    }
    // Create a new user role on the fly.
    policy := &acl_proto.ApiClientACL{
    Roles: self.default_roles,
    }
    services.LogAudit(r.Context(),
    self.config_obj, username, "Automatic User Creation",
    ordereddict.NewDict().
    Set("roles", self.default_roles).
    Set("remote", r.RemoteAddr))
    // Use the super user principal to actually add the
    // username so we have enough permissions.
    err = users_manager.AddUserToOrg(r.Context(), services.AddNewUser,
    utils.GetSuperuserName(self.config_obj), username,
    []string{"root"}, policy)
    if err != nil {
    http.Error(w,
    fmt.Sprintf("authorization failed: automatic user creation: %v", err),
    http.StatusUnauthorized)
    return
    }
    user_record, err = users_manager.GetUser(r.Context(), username, username)
    if err != nil {
    http.Error(w,
    fmt.Sprintf("Failed creating user for %v: %v", username, err),
    http.StatusUnauthorized)
    return
    }
    }
    // Does the user have access to the specified org?
    err = CheckOrgAccess(self.config_obj, r, user_record)
    if err != nil {
    services.LogAudit(r.Context(),
    self.config_obj, user_record.Name, "Unauthorized username",
    ordereddict.NewDict().
    Set("remote", r.RemoteAddr).
    Set("status", http.StatusUnauthorized))
    http.Error(w,
    fmt.Sprintf("authorization failed: %v", err),
    http.StatusUnauthorized)
    return
    }

Instead of reusing the gui.authenticator.default_roles_for_unknown_users field we introduced a new saml_user_roles field to ensure privilege separation between gui users and client certificates.

@CLAassistant
Copy link

CLAassistant commented Jun 11, 2024

CLA assistant check
All committers have signed the CLA.

@ZauberNerd ZauberNerd force-pushed the saml-automatic-account-creation branch from fd31d25 to 213d390 Compare June 11, 2024 12:06
ZauberNerd added a commit to HiSolutions/velociraptor-docs that referenced this pull request Jun 11, 2024
@scudette
Copy link
Contributor

Thanks for this pr. I was a bit reluctant to automate user creation based on saml or oauth because that can have unintended effects. For example any user that's authenticated by saml may automatically get access to velociraptor.

Typically the teams that manage velociraptor access are not the same as teams that manage user accounts and I can see accidents happening with users automatically created in velociraptor.

I guess if it's very clearly documented and people understand the risk then it's fine but how likely is it to have this kind of mistake?

ZauberNerd added a commit to HiSolutions/velociraptor-docs that referenced this pull request Jun 11, 2024
ZauberNerd added a commit to HiSolutions/velociraptor-docs that referenced this pull request Jun 11, 2024
With this change we are able to automatically create users for SAML
authenticated users and assign them roles.
Before this change, users had to be created manually on the velociraptor
server, even when using SAML authentication, which lead to double book
keeping.

Based on the work in the certs authenticator module:
- https://github.com/Velocidex/velociraptor/blob/858e2e4a1c597c9dfd1d0389f84ac2186eb3bae7/api/authenticators/certs.go#L168-L223

Instead of reusing the gui.authenticator.default_roles_for_unknown_users
field we introduced a new saml_user_roles field to ensure privilege
separation between gui users and client certificates.

Co-Authored-By: Deniz Adrian <[email protected]>
@ZauberNerd ZauberNerd force-pushed the saml-automatic-account-creation branch from 213d390 to f67dbc0 Compare June 11, 2024 13:10
@ZauberNerd
Copy link
Contributor Author

@scudette if the authenticator.saml_user_roles config is not set, then no users will be created. You could then create administrator accounts manually and afterwards enable SAML role assignment with less privileged accounts.
I have updated the documentation and comments to mention this behavior. Do you think that is good enough?

In a future iteration, we could think about adding a configuration setting to allow using other SAML attributes to map for example groups to policies. What do you think?

@scudette
Copy link
Contributor

This is what I was asking being not very familiar with SAML administration - does SAML allow the application to be assigned to a user or does it mean that anyone who is able to authenticate to SAML anywhere in the domain can also log into the velociraptor app (and therefore get automatically created?)

With OAUTH this is actually the case - for example, if we use Google to implement OAuth then literally anyone who can authenticate to Google (e.g. random Gmail user) will be able to return from the oauth handler and pass into AuthenticateUserHandler() callback. They will normally be stopped in the user account check, but if the expectation is that an account will be automatically provisioned then anyone that creates a gmail account will get a login!

@zined
Copy link
Contributor

zined commented Jun 11, 2024

This is what I was asking being not very familiar with SAML administration - does SAML allow the application to be assigned to a user or does it mean that anyone who is able to authenticate to SAML anywhere in the domain can also log into the velociraptor app (and therefore get automatically created?)

from my (probably rather limited) experience with SAML identity providers, "filtering" users per application seems to be a very standard functionality included almost everywhere. At least AD-FS/Keycloak/AWS Identity Center all have logic to "map" users to an application based on group-memberships or other factors, therefore limiting permitted users to a subset of all users already. From my point of view, having any mapping/filtering logic inside the IDP vs. inside the SP is highly desirable to (a) keep it simple and (b) not have a lot of distributed filter mechanisms throughout your application landscape, but rather in a central place.

EDIT: don't get me wrong, i totally believe having logic to map separate roles inside velociraptor to users based on SAML attributes makes a lot of sense in a more complex organizational structure where you need different users to have different roles inside your velociraptor setup, but I believe it would make sense to implement this in a separate PR.

@scudette scudette merged commit 66960c3 into Velocidex:master Jun 12, 2024
3 checks passed
@ZauberNerd ZauberNerd deleted the saml-automatic-account-creation branch June 12, 2024 07:20
scudette pushed a commit that referenced this pull request Jun 30, 2024
With this change we are able to automatically create users for SAML
authenticated users and assign them roles.
Before this change, users had to be created manually on the velociraptor
server, even when using SAML authentication, which lead to double book
keeping.

Based on the work in the certs authenticator module:
-
https://github.com/Velocidex/velociraptor/blob/858e2e4a1c597c9dfd1d0389f84ac2186eb3bae7/api/authenticators/certs.go#L168-L223

Instead of reusing the gui.authenticator.default_roles_for_unknown_users
field we introduced a new saml_user_roles field to ensure privilege
separation between gui users and client certificates.

Co-authored-by: Deniz Adrian <[email protected]>
scudette pushed a commit that referenced this pull request Jun 30, 2024
With this change we are able to automatically create users for SAML
authenticated users and assign them roles.
Before this change, users had to be created manually on the velociraptor
server, even when using SAML authentication, which lead to double book
keeping.

Based on the work in the certs authenticator module:
-
https://github.com/Velocidex/velociraptor/blob/858e2e4a1c597c9dfd1d0389f84ac2186eb3bae7/api/authenticators/certs.go#L168-L223

Instead of reusing the gui.authenticator.default_roles_for_unknown_users
field we introduced a new saml_user_roles field to ensure privilege
separation between gui users and client certificates.

Co-authored-by: Deniz Adrian <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants