Idea #20663
closedConfiguration to limit resources managed by arvados-login-sync
Description
arvados-login-sync
is willing to modify the following resources:
- User accounts
- Group additions
- Group removals
- User SSH authorized keys
- User Arvados API credentials
This is very convenient if you have no other system management tools, but many organizations have other ways to manage these resources, and do not want arvados-login-sync
interfering with their existing stacks. It should be possible to configure arvados-login-sync
to manage a subset of these resources.
Proposed solution:
- Arvados configuration grows a setting that lists dedicated keys representing what resources
arvados-login-sync
is allowed to manage. The default is all of them, for backwards compatibility. - This configuration is published through the API server.
arvados-login-sync
reads this configuration from the API server.- If the configuration says
arvados-login-sync
is not allowed to manage a resource, then it does not take any action that might modify that resource. It assumes that another tool is managing that. - If
arvados-login-sync
cannot do a management task that it should do but cannot because of missing system resources, it logs that fact and does not make any changes associated with that user account. For example, ifarvados-login-sync
is configured to manage SSH keys and API tokens, and these resources exist for a user+VM but that user does not exist on the VM, it logs the fact that it cannot manage a nonexistent user and skips it.
Looking ahead a bit: the reason to make this configuration available through the API is so that client tools can adapt their management interfaces to match. For example, if we do this, there can be a follow-up story so that Workbench 2 does not present management UI for resources that are not managed by arvados-login-sync
.
Updated by Peter Amstutz over 1 year ago
- Story points set to 2.0
- Target version changed from Future to To be scheduled
Updated by Peter Amstutz over 1 year ago
- Target version changed from To be scheduled to Development 2023-07-05 sprint
Updated by Brett Smith over 1 year ago
20663-login-sync-config @ ee35d22df94f1745f97c17f3171e8663fa2e375e - developer-run-tests: #3716
Note I would like to do some manual testing to make sure there are no obvious regressions, etc. But hopefully that wouldn't require any major changes.
Updated by Lucas Di Pentima over 1 year ago
Re-running failed test pipeline: developer-run-tests-remainder: #3905
Updated by Brett Smith over 1 year ago
Brett Smith wrote in #note-5:
Note I would like to do some manual testing to make sure there are no obvious regressions, etc. But hopefully that wouldn't require any major changes.
After too much wrangling with RVM and Gems this is done, it works as expected.
Updated by Lucas Di Pentima over 1 year ago
Some questions:
- Knowing that this is out of scope, but just in case you think is simple enough to add: How about a config knob
Users.SyncIgnoredGroups
to allow the site admin team to avoid some potentially problematic groups (like 'root', 'sudo', etc) in case the Arvados admin team is made of different people? - Haven't looked at it in detail but I'm seeing that there's some (not much, unfortunately) testing on login-sync, do you think it could be expanded to test this new behaviors?
- IIRC you asked about API versioning. Not sure if we want it for this branch but just in case, you could change the timestamp at
arvados/services/api/app/controllers/arvados/v1/schema_controller.rb:L39
.
Updated by Brett Smith over 1 year ago
20663-login-sync-config @ 9b4f22418bc26d57e4b9d4a0ba9ef3c4e34a2e51 - developer-run-tests: #3718
Lucas Di Pentima wrote in #note-8:
- Knowing that this is out of scope, but just in case you think is simple enough to add: How about a config knob
Users.SyncIgnoredGroups
to allow the site admin team to avoid some potentially problematic groups (like 'root', 'sudo', etc) in case the Arvados admin team is made of different people?
Sure, added in the most recent commit.
- Haven't looked at it in detail but I'm seeing that there's some (not much, unfortunately) testing on login-sync, do you think it could be expanded to test this new behaviors?
I don't think it's worth it, unfortunately. arvados-login-sync isn't written to be testable: it's a giant oneshot script that does all of its system operations directly. The good way to fix this would be to modify the script to split up functionality so that it becomes testable. The alternative is to write a huge amount of test scaffolding.
I am guessing at some point semi-soon-ish we will want to rewrite this service in either Go or Python, just because those are our first-class SDKs. I think that would be a good time to make the component testable too.
- IIRC you asked about API versioning. Not sure if we want it for this branch but just in case, you could change the timestamp at
arvados/services/api/app/controllers/arvados/v1/schema_controller.rb:L39
.
I don't think it's necessary this time around. The changes in the branch can handle talking to an older API server no problem: it has defaults for all the configuration that match its original behavior.
Updated by Brett Smith over 1 year ago
Brett Smith wrote in #note-9:
Lucas Di Pentima wrote in #note-8:
- Knowing that this is out of scope, but just in case you think is simple enough to add: How about a config knob
Users.SyncIgnoredGroups
to allow the site admin team to avoid some potentially problematic groups (like 'root', 'sudo', etc) in case the Arvados admin team is made of different people?Sure, added in the most recent commit.
Per standup discussion, updated the default value for this setting to tighten security. Now 20663-login-sync-config @ aad254eb85755d41927fe1809cb52c65bb8aac20 - developer-run-tests: #3719
Updated by Brett Smith over 1 year ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|b5dad64d1faa5063482db0d33d22805912abdda6.