Page MenuHomePhabricator

Auth - add "manage providers" capability
ClosedPublic

Authored by btrahan on Jan 12 2015, 10:13 PM.
Tags
None
Referenced Files
Unknown Object (File)
Jan 29 2024, 11:18 PM
Unknown Object (File)
Jan 20 2024, 2:05 AM
Unknown Object (File)
Jan 15 2024, 8:59 PM
Unknown Object (File)
Jan 15 2024, 8:59 PM
Unknown Object (File)
Dec 24 2023, 2:27 PM
Unknown Object (File)
Dec 17 2023, 4:24 AM
Unknown Object (File)
Nov 30 2023, 4:45 AM
Unknown Object (File)
Nov 26 2023, 12:46 AM
Subscribers

Details

Summary

Ref T6947.

Test Plan

toggled setting in application settings and changes stuck. set policy to admin user a only and could not add a provider as a admin user b.

Diff Detail

Repository
rP Phabricator
Branch
T6947
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 3798
Build 3810: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

btrahan retitled this revision from to Auth - add "manage providers" capability.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
epriestley edited edge metadata.

One inline. We should also:

  • Check in PhabricatorAuthDisableController.
  • Maybe setDisabled(true) on the "Add Authentication Provider" link on the List view if you don't have the capability, so the button greys out. Likewise with the "disable" "X" on the list view.
  • After we default to "Admin", we could remove shouldRequireAdmin() from PhabricatorAuthProviderConfigController. I'm not 100% sure this has use cases -- it would let you create a more open policy around provider management -- but it's more consistent with how other policies work.
src/applications/auth/application/PhabricatorAuthApplication.php
149

This should default to PhabricatorPolicies::POLICY_ADMIN here.

This revision is now accepted and ready to land.Jan 12 2015, 10:19 PM
btrahan edited edge metadata.

changes as requested

This revision was automatically updated to reflect the committed changes.

The handleReqeust(AphrontRequest $request) cleanups are also really nice, I'm glad we made that change.

Yeah, the code definitely looks better this way... I figure I'll keep nibbling and eventually we can get more serious about conversion.