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
F14094872: D11401.diff
Mon, Nov 25, 6:04 PM
F14094733: D11401.diff
Mon, Nov 25, 5:00 PM
Unknown Object (File)
Sun, Nov 24, 2:03 AM
Unknown Object (File)
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
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
Branch
T6949
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 3857
Build 3869: [Placeholder Plan] Wait for 30 Seconds

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
5

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
5

I think "users" is a sensible default.

11

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 27382

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