Page MenuHomePhabricator
Differential D11401

OAuthServer - hide client secret behind a "View Secret" action
ClosedPublic
Actions

Authored by btrahan on Jan 14 2015, 10:51 PM.
Tags
None
Referenced Files
F14076625: D11401.diff
Thu, Nov 21, 6:55 PM
Unknown Object (File)
Mon, Nov 18, 5:17 AM
Unknown Object (File)
Thu, Nov 14, 3:12 AM
Unknown Object (File)
Sun, Nov 10, 12:38 AM
Unknown Object (File)
Wed, Nov 6, 7:41 AM
Unknown Object (File)
Fri, Nov 1, 6:13 AM
Unknown Object (File)
Wed, Oct 23, 11:03 PM
Unknown Object (File)
Oct 21 2024, 7:43 AM
View All Files
Subscribers
epriestley
Korvin
Tokens
"Doubloon" token, awarded by epriestley.

Details

Reviewers
epriestley
Maniphest Tasks
T6949: Hide OAuth server secrets behind "Show Secret" to defuse screenshot/over-the-shoulder leaks
Commits
Restricted Diffusion Commit
rP1cc81b1d0ae4: OAuthServer - hide client secret behind a "View Secret" action
Summary

...also adds policies on who can view and who can edit an action. Fixes T6949.

Test Plan

viewed a secret through the new UI and it worked

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

btrahan updated this revision to Diff 27382.Jan 14 2015, 10:51 PM
btrahan added a task: T6949: Hide OAuth server secrets behind "Show Secret" to defuse screenshot/over-the-shoulder leaks.
btrahan retitled this revision from to OAuthServer - hide client secret behind a "View Secret" action.
btrahan updated this object.
btrahan edited the test plan for this revision. (Show Details)
btrahan added a reviewer: epriestley.
Herald added subscribers: epriestley, Korvin. · View Herald TranscriptJan 14 2015, 10:51 PM
btrahan added inline comments.Jan 14 2015, 10:52 PM
resources/sql/autopatches/20150114.oauthserver.client.policy.sql
6

these could maybe be admin since no one is using this probably so its cool to break behavior? i'd also have to make the default be admin in the app code.

epriestley accepted this revision.Jan 15 2015, 1:08 AM
epriestley edited edge metadata.
epriestley added inline comments.
resources/sql/autopatches/20150114.oauthserver.client.policy.sql
6

I think "users" is a sensible default.

12

This could be editPolicy = creatorPHID (only allow the user who created the object to edit it), which I think mostly preserves existing behavior.

src/applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php
34

This should probably set defaults -- I guess USERS for view, and the viewer's PHID for edit?

This revision is now accepted and ready to land.Jan 15 2015, 1:08 AM
epriestley awarded a token.Jan 15 2015, 1:08 AM
btrahan updated this revision to Diff 27385.Jan 15 2015, 1:26 AM
btrahan edited edge metadata.

changes as requested. thanks!

Closed by commit rP1cc81b1d0ae4: OAuthServer - hide client secret behind a "View Secret" action (authored by btrahan, committed by btrahan). · Explain WhyJan 15 2015, 1:27 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 27386

View Options

resources/sql/autopatches/20150114.oauthserver.client.policy.sql

Loading...
View Options

src/__phutil_library_map__.php

Loading...
View Options

src/applications/oauthserver/application/PhabricatorOAuthServerApplication.php

Loading...
View Options

src/applications/oauthserver/controller/client/PhabricatorOAuthClientEditController.php

Loading...
View Options

src/applications/oauthserver/controller/client/PhabricatorOAuthClientSecretController.php

Loading...
View Options

src/applications/oauthserver/controller/client/PhabricatorOAuthClientViewController.php

Loading...
View Options

src/applications/oauthserver/storage/PhabricatorOAuthServerClient.php

Loading...
Phabricator · Built by Phacility