Skip to content
This repository has been archived by the owner on Sep 23, 2023. It is now read-only.

Allow site config to be overridden by trusted users #112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

edg2s
Copy link
Collaborator

@edg2s edg2s commented Sep 17, 2020

Fixes #19

@edg2s edg2s requested a review from MatmaRex September 17, 2020 14:05
@edg2s
Copy link
Collaborator Author

edg2s commented Sep 17, 2020

This does away with the JSON approach of #60 as that adds no real security.

Instead we can maintain a list of trusted OAuth accounts that will be allowed to use the site config feature.

Anyone who is V+2 can be added to this list, as V+2ers can already execute arbitrary code on the server.

To begin with we should use the regex feature to allow / \(WMF\)$/ as I believe these names are protected.

@DannyS712
Copy link
Contributor

Would love to be able to do this, but how exactly does the code detect if a user is V+2? OAuth connects to SUL, and being able to vote V+2 is based on gerrit permissions on an account that may not be connected.

@edg2s
Copy link
Collaborator Author

edg2s commented Sep 27, 2020

This patch requires us to manually maintain a list of trusted users.

@MatmaRex
Copy link
Owner

MatmaRex commented Oct 2, 2020

I like this but I'd also like to avoid maintaining that list for the rest of my life, hmm…

@edg2s
Copy link
Collaborator Author

edg2s commented Oct 3, 2020

One could host the user list in a Gerrit repo, then v+2ers could add themselves.

@DannyS712
Copy link
Contributor

One could host the user list in a Gerrit repo, then v+2ers could add themselves.

That would also require CR+2, though that should not be a problem since the two usually go together.
That would also ensure that the list only includes those that want to be included and plan to use the tool

Sounds like a good idea to me. New repo, something like mediawiki/tools/patchdemoconfig (in case the actual code from this ever moves, not using patchdemo) inheriting from mediawiki/* with a simple text file that users are told they can self-merge changes to, file just holds a list of allowed user names

@edg2s edg2s force-pushed the config branch 2 times, most recently from 776e4b8 to 75dba6b Compare December 6, 2021 15:57
// Same as above, but regexes e.g. / \(WMF\)$/
'configurersMatch' => [],
// Instructions to request 'configurers' user status, e.g. "File a request <a href=...>here</a>."
'configurersRequestHtml' => '',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested config for us:
Request approval by creating a <a href="https://github.com/MatmaRex/patchdemo/issues/new">new issue</a>.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide a way to override configs
3 participants