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

Option to restrict jobs to internal users only #419

Merged
merged 5 commits into from
Jan 19, 2024
Merged

Option to restrict jobs to internal users only #419

merged 5 commits into from
Jan 19, 2024

Conversation

rebkwok
Copy link
Contributor

@rebkwok rebkwok commented Jan 17, 2024

Add minimal restrictions that can be applied to categories of jobs, to restrict the entire category to internal users.

For every slack message event sent, the slack API includes a user id, which we can look up to get the value of is_restricted, which is True if the user is a guest (i.e. external) user.

I've updated the job config to restrict the jobs that I think we care about - op, fdaaa, funding and techsupport (techsupport for the out of office on/off commands, not the rota). For the others, I don't think we're concerned about external users seeing the project boards, logs, or outputchecking rota.

To allow namespaces to refuse to run jobs for guest users (using the
`is_restricted` field returned by the slack api). For now we make it
applicable to the entire namespace, not to individual commands.
Copy link
Contributor

@inglesp inglesp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. What do you think about jobs being restricted by default, so that we have to explicitly opt in to allowing external users to run them?

@rebkwok
Copy link
Contributor Author

rebkwok commented Jan 19, 2024

Looks good. What do you think about jobs being restricted by default, so that we have to explicitly opt in to allowing external users to run them?

I did think about that, but decided maybe that went against our usual open-by-default stance. It would be easy to come back to, so I think I'll just leave it as is for now

@rebkwok rebkwok merged commit 22906a2 into main Jan 19, 2024
6 checks passed
@rebkwok rebkwok deleted the restrict-jobs branch January 19, 2024 10:16
@evansd
Copy link
Contributor

evansd commented Jan 19, 2024

FWIW, my instinct would very much be on the side on restricting commands by default and then explicitly marking the safe ones. I don't think this needs doing immediately but maybe next time we want to add a restricted command we should invert the logic.

maybe that went against our usual open-by-default stance

I think that applies to sharing information and working in the open, not to giving people permission to do stuff.

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

3 participants