Add actions to group/scope auth #2147

pschoenfelder · 2020-10-02T19:21:08Z

Some requests need to be runnable ONLY by someone other than the group owner for compliance purposes. For example, Team A would write and deploy a sensitive request, and only Team B could run it (or bounce, scale, kill tasks, or pause).

This provides a more generic solution in the form of an action permissions override map.

ssalinas · 2020-10-02T19:25:19Z

SingularityBase/src/main/java/com/hubspot/singularity/SingularityAuthorizationScope.java

@@ -4,5 +4,6 @@
  READ,
  WRITE,
  ADMIN,
-  DEPLOY
+  DEPLOY,
+  EXEC // run, bounce, kill, pause, scale


Since we are adding granularity, would it be too much harder to actually make each of these their own separate scope? Would let us much more easily divide things up later if needed

ssalinas · 2020-10-02T19:26:49Z

...ityService/src/main/java/com/hubspot/singularity/auth/SingularityGroupsScopesAuthorizer.java

@@ -211,6 +144,19 @@ public boolean isAuthorizedForRequest(
    }
  }

+  private void warnJita(


just FYI that this will have some merge conflicts with #2141

ssalinas · 2020-10-02T19:29:34Z

...ityService/src/main/java/com/hubspot/singularity/auth/SingularityGroupsScopesAuthorizer.java

+        return Sets.union(scopesConfiguration.getRead(), scopesConfiguration.getWrite());
+      case WRITE:
+        return scopesConfiguration.getWrite();
+      case DEPLOY:


If it makes it simpler in the overall implementation, I'm fine with having the DEPLOY scope behave similar to the new ones (i.e. falls under WRITE unless otherwise overriden by a request). I had originally separated out DEPLOY for future use with our internal automation, but it isn't turned on or enforced by default

Do you mean change DEPLOY to not fall back to use WRITE? I set up EXEC so that it doesn't fall back to write, but that the request group gets EXEC privs unless explicitly removed by the overrides.

We're going to have to add all these scopes for all the appropriate people anyways, it would be easy to add DEPLOY for them too

ssalinas · 2020-10-07T13:22:46Z

SingularityBase/src/main/java/com/hubspot/singularity/api/SingularityUpdateGroupsRequest.java

@@ -13,20 +15,25 @@
  private final Optional<String> group;
  private final Set<String> readWriteGroups;
  private final Set<String> readOnlyGroups;
+  private final Optional<Map<String, Set<SingularityAuthorizationScope>>> groupScopeOverrides;


should this also be renamed/updated to actionPermissions?

ssalinas · 2020-10-07T13:26:34Z

SingularityService/src/main/java/com/hubspot/singularity/config/ScopesConfiguration.java

@@ -14,6 +14,9 @@
  @NotEmpty
  private Set<String> read = Collections.singleton("SINGULARITY_READONLY");

+  @NotEmpty
+  private Set<String> exec = Collections.singleton("SINGULARITY_EXEC");


can probably remove this since we went with the actions strategy instead

ssalinas · 2020-10-07T13:28:00Z

SingularityBase/src/main/java/com/hubspot/singularity/SingularityUserFacingAction.java

+package com.hubspot.singularity;
+
+public enum SingularityUserFacingAction {
+  RUN_REQUEST,


naming nits:

Not sure we need the _REQUEST on each of these. Will likely be easier for users to remember in their configs without it

Maybe RUN -> EXEC/EXECUTE?

Agreed on dropping _REQUEST.

I feel like RUN matches up with the ui ("Run Now"), where EXEC/EXECUTE isn't a term users typically use, so it may be easier to remember.

ssalinas · 2020-10-07T13:28:24Z

SingularityBase/src/main/java/com/hubspot/singularity/SingularityUserFacingAction.java

+  PAUSE_REQUEST,
+  SCALE_REQUEST,
+  KILL_TASK,
+  BOUNCE_TASK,


Is bounce task any different from KILL_TASK? I don't see it used anywhere

ssalinas · 2020-10-07T13:30:43Z

...ityService/src/main/java/com/hubspot/singularity/auth/SingularityGroupsScopesAuthorizer.java

+
+    if (request.getActionPermissions().isPresent()) {
+      request
+        .getActionPermissions()


is this needed here still? I think the action permissions are all write-based (and also not SingularityAuthorizationScope objects)

Good catch - missed this one

ssalinas

Overall I like the actions strategy, seems a lot cleaner. One open question is if a write-jita should allow a user to bypass not being the proper action group. I think the answer should be yes there, but open to opinions. Definitely +1 for unit tests here. It's much easier to tell that all the authorizer bits are working with those than by reading the code alone

pschoenfelder · 2020-10-07T15:44:18Z

re: write-jita — Could we disallow jita action bypass by default, and leave it up to the user to add the jita group to the action permissions if that should be allowed? It's less obvious for users, but more protective and explicit. If not, then I would assume that write-jita should allow bypassing the action permissions.

ssalinas · 2020-10-08T13:06:56Z

I think the fact that we have the audit events that Aditya added should be enough on the jita front to just allow jita through to everything. One of the main requirements on this was that any actions normally not allowed would at least be logged/audited

pschoenfelder · 2020-10-20T15:33:40Z

Added tests that validate JITA support for actions

ssalinas · 2020-10-20T19:07:13Z

🚢

pschoenfelder added 8 commits October 1, 2020 17:34

Refactor and additional test

0e13d83

WIP refactor

ba7b38b

Add group permission overrides

6a818db

Rename

483bf57

Add exec scope

09fe249

Add test

61ee7af

Update groups request

c521145

Update auth scopes

4a93dfa

ssalinas reviewed Oct 2, 2020

View reviewed changes

pschoenfelder added 2 commits October 2, 2020 16:22

Remove exec and deploy

f6de563

Add actions

06f0216

pschoenfelder changed the title ~~Add EXEC scope~~ Add actions to group/scope auth Oct 6, 2020

ssalinas reviewed Oct 7, 2020

View reviewed changes

annotation fix

e23de13

PR changes and update request changes

132efc4

Additional JITA tests

f3102b3

Merge branch 'master' of github.com:HubSpot/Singularity into scopes

00ce742

pschoenfelder merged commit c1454ff into master Oct 20, 2020

pschoenfelder deleted the scopes branch October 20, 2020 19:09

ssalinas added this to the 1.4.0 milestone Jan 5, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add actions to group/scope auth #2147

Add actions to group/scope auth #2147

pschoenfelder commented Oct 2, 2020 •

edited

ssalinas Oct 2, 2020

ssalinas Oct 2, 2020

ssalinas Oct 2, 2020

pschoenfelder Oct 2, 2020

pschoenfelder Oct 2, 2020

ssalinas Oct 7, 2020

ssalinas Oct 7, 2020

ssalinas Oct 7, 2020

pschoenfelder Oct 7, 2020

ssalinas Oct 7, 2020

ssalinas Oct 7, 2020

pschoenfelder Oct 7, 2020

ssalinas left a comment

pschoenfelder commented Oct 7, 2020

ssalinas commented Oct 8, 2020

pschoenfelder commented Oct 20, 2020

ssalinas commented Oct 20, 2020

Add actions to group/scope auth #2147

Add actions to group/scope auth #2147

Conversation

pschoenfelder commented Oct 2, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssalinas left a comment

Choose a reason for hiding this comment

pschoenfelder commented Oct 7, 2020

ssalinas commented Oct 8, 2020

pschoenfelder commented Oct 20, 2020

ssalinas commented Oct 20, 2020

pschoenfelder commented Oct 2, 2020 •

edited