1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-28 09:42:41 +01:00
Commit graph

22 commits

Author SHA1 Message Date
vrana
5ad526942b Convert AphrontPanelView to safe HTML (except children)
Summary: Fixes some double escaping and potential XSS.

Test Plan: Looked at homepage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4917
2013-02-13 10:30:32 -08:00
vrana
58b6e2cac6 Convert AphrontDialogView to safe HTML
Summary:
Done by searching for `AphrontDialogView` and then `appendChild()`.

Also added some `pht()`.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4882
2013-02-09 15:11:35 -08:00
vrana
e4736bf977 Avoid double escaping in error view title
Summary: Searched for `AphrontErrorView` and then for `setTitle()`.

Test Plan: None.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2432

Differential Revision: https://secure.phabricator.com/D4880
2013-02-09 15:11:33 -08:00
epriestley
11bb8db970 Merge branch 'master' into phutil_tag
(Sync.)
2013-02-07 08:08:01 -08:00
Toby Hughes
1188876ea9 Fix exception in OAuthServerAuthController
Summary:
We've been building a Jenkins plugin that allows you to use your Phabricator login details in Jenkins using the inbuilt OAuthServer. I noticed that when making a request to /oauthserver/auth/?client_id=&response_type=code I get an error back from the server.

I've traced this down to two bugs in PhabricatorOAuthServerAuthController, the first causes a null value error on $access_token_uri, and the second fails on userHasAuthorizedClient without a $scope array.

Test Plan: Go to /oauthserver/auth/?client_id=<client_id>&response_type=code and get a valid authorization code back

Reviewers: epriestley, btrahan

Reviewed By: btrahan

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4808
2013-02-06 09:00:01 -08:00
vrana
3c1b8df8ae Convert simple phutil_render_tag() to phutil_tag()
Summary: Done manually.

Test Plan: Loaded homepage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4509
2013-01-24 19:30:50 -08:00
vrana
20768d65d5 Convert phutil_render_tag(X, Y, '...') to phutil_tag
Summary:
Created with spatch:

  lang=diff
  - phutil_render_tag
  + phutil_tag
    (X, Y, '...')

Then searched for `&` and `<` in the output and replaced them.

Test Plan: Loaded homepage.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4503
2013-01-24 19:20:27 -08:00
vrana
48561a8b1f Convert phutil_render_tag(X, Y, phutil_escape_html(Z)) to phutil_tag
Summary:
Created with spatch:

  lang=diff
  - phutil_render_tag
  + phutil_tag
    (X, Y,
  - phutil_escape_html(
    Z
  - )
    )

Test Plan: Loaded homepage

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4501
2013-01-24 19:08:55 -08:00
Chad Little
38626dce64 Remove spacer from sidebars.
Summary: This removes all calls to addSpacer and the method. We were applying it inconsistently and it was causing spacing issues with redesigning the sidenav. My feeling is we can recreate the space in CSS if the design dictates, which would apply it consistently.

Test Plan: Go to Applications, click on every application.

Reviewers: epriestley, btrahan

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D4420
2013-01-13 08:17:12 -08:00
vrana
ef85f49adc Delete license headers from files
Summary:
This commit doesn't change license of any file. It just makes the license implicit (inherited from LICENSE file in the root directory).

We are removing the headers for these reasons:

- It wastes space in editors, less code is visible in editor upon opening a file.
- It brings noise to diff of the first change of any file every year.
- It confuses Git file copy detection when creating small files.
- We don't have an explicit license header in other files (JS, CSS, images, documentation).
- Using license header in every file is not obligatory: http://www.apache.org/dev/apply-license.html#new.

This change is approved by Alma Chao (Lead Open Source and IP Counsel at Facebook).

Test Plan: Verified that the license survived only in LICENSE file and that it didn't modify externals.

Reviewers: epriestley, davidrecordon

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2035

Differential Revision: https://secure.phabricator.com/D3886
2012-11-05 11:16:51 -08:00
epriestley
66cee129b6 Remove all code referencing old tab navigation
Summary:
  - Add getHelpURI() to PhabricatorApplication for application user guides.
  - Add a new "help" icon menu item and skeletal Diviner application.
  - Move help tabs to Applications where they exist, document the other ones that don't exist yet.
  - Grep for all tab-related stuff and delete it.

Test Plan: Clicked "help" for some apps. Clicked around randomly in a bunch of other apps.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3267
2012-08-13 15:28:41 -07:00
epriestley
ed4a155c91 Rename "IDPaged" to "CursorPaged", "executeWithPager" to "executeWith[Cursor|Offset]Pager"
Summary:
I'm trying to make progress on the policy/visibility stuff since it's a blocker for Wikimedia.

First, I want to improve Projects so they can serve as policy groups (e.g., an object can have a visibility policy like "Visible to: members of project 'security'"). However, doing this without breaking anything or snowballing into a bigger change is a bit awkward because Projects are name-ordered and we have a Conduit API which does offset paging. Rather than breaking or rewriting this stuff, I want to just continue offset paging them for now.

So I'm going to make PhabricatorPolicyQuery extend PhabricatorOffsetPagedQuery, but can't currently since the `executeWithPager` methods would clash. These methods do different things anyway and are probably better with different names.

This also generally improves the names of these classes, since cursors are not necessarily IDs (in the feed case, they're "chronlogicalKeys", for example). I did leave some of the interals as "ID" since calling them "Cursor"s (e.g., `setAfterCursor()`) seemed a little wrong -- it should maybe be `setAfterCursorPosition()`. These APIs have very limited use and can easily be made more consistent later.

Test Plan: Browsed around various affected tools; any issues here should throw/fail in a loud/obvious way.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3177
2012-08-07 11:54:06 -07:00
vrana
2e484e257d Fix lint errors found by Nemo
Summary:
See also:

- https://github.com/tpyo/amazon-s3-php-class/pull/33
- https://github.com/stripe/stripe-php/pull/13

Test Plan: Ran a script analyzing sources by HPHP.

Reviewers: btrahan, jungejason, epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2713
2012-06-11 19:09:42 -07:00
vrana
6cc196a2e5 Move files in Phabricator one level up
Summary:
- `kill_init.php` said "Moving 1000 files" - I hope that this is not some limit in `FileFinder`.
- [src/infrastructure/celerity] `git mv utils.php map.php; git mv api/utils.php api.php`
- Comment `phutil_libraries` in `.arcconfig` and run `arc liberate`.

NOTE: `arc diff` timed out so I'm pushing it without review.

Test Plan:
/D1234
Browsed around, especially in `applications/repository/worker/commitchangeparser` and `applications/` in general.

Auditors: epriestley

Maniphest Tasks: T1103
2012-06-01 12:32:44 -07:00
epriestley
09c8af4de0 Upgrade phabricator to libphutil v2
Summary: Mechanical changes from D2588. No "Class.php" moves yet.

Test Plan: See D2588.

Reviewers: vrana, btrahan, jungejason

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2591
2012-05-30 14:26:29 -07:00
Bob Trahan
09172a1937 Add pagers to server clients and client authorizations in OAuth Server GUI
Summary: ...also make the pager usage in ChatLog use the nice formatWhereClause functionality

Test Plan: set $page_size = 2 and paged around the data a bit

Reviewers: epriestley

Reviewed By: epriestley

CC: aran

Maniphest Tasks: T905

Differential Revision: https://secure.phabricator.com/D2106
2012-04-04 17:51:16 -07:00
Bob Trahan
8f2c426ff2 Fix OAuth Client Authorization bugs
Summary: ajtrichards reported an error creating a brand new authorization. fixed that and generally made this flow work well

Test Plan:
- created a fresh test client
-- noted "new=<PHID>" with appropriate highlighting
- visited http://phabricator.dev/oauthserver/auth/?client_id=PHID-OASC-jwgdrqdpzomtxyg3q3yf&response_type=code&scope=offline_access
-- clicked "cancel", verified result
-- clicked "approve", verfied result
- visited http://phabricator.dev/oauthserver/auth/?client_id=PHID-OASC-jwgdrqdpzomtxyg3q3yf&response_type=code&scope=whoami
-- noted got the dialog
-- noted that it had the union of desired and existing so user could update it properly! (NB - its up to the client to react to whatever specific scope(s) the user decides to grant)
-- noted it actually updated when I hit "approve"

Reviewers: epriestley, ajtrichards

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T933

Differential Revision: https://secure.phabricator.com/D1775
2012-03-05 13:27:20 -08:00
Bob Trahan
cd30946d60 OAuth Server -- add a doc and link to it in a few places
Summary: add a big ole HELP tab and make "scope" link to the specific
sub-section about scope

Test Plan:
read my doc a few times, it basically english
verified links looked correct and should work right once this is all in
production

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T910

Differential Revision: https://secure.phabricator.com/D1752
2012-03-01 17:27:52 -08:00
Bob Trahan
0327a5fc69 OAuthServer polish and random sauce
Summary:
This diff makes the OAuthServer more compliant with the spec by
- making it return well-formatted error codes with error types from the spec.
- making it respect the "state" variable, which is a transparent variable the
client passes and the server passes back
- making it be super, duper compliant with respect to redirect uris
-- if specified in authorization step, check if its valid relative to the client
registered URI and if so save it
-- if specified in authorization step, check if its been specified in the access
step and error if it doesn't match or doesn't exist
-- note we don't make any use of it in the access step which seems strange but
hey, that's what the spec says!
This diff makes the OAuthServer suck less by
- making the "cancel" button do something in the user authorization flow
- making the client list view and client edit view be a bit more usable around
client secrets
- fixing a few bugs I managed to introduce along the way

Test Plan:
- create a test phabricator client, updated my conf, and then linked and
unlinked phabricator to itself
- wrote some tests for PhabricatorOAuthServer -- they pass!
-- these validate the various validate URI checks
- tried a few important authorization calls
--
http://phabricator.dev/oauthserver/auth/?client_id=X&state=test&redirect_uri=http://www.evil.com
--- verified error'd from mismatching redirect uri's
--- verified state parameter in response
--- verified did not redirect to client redirect uri
-- http://phabricator.dev/oauthserver/auth/?client_id=X w/ existing
authorization
--- got redirected to proper client url with error that response_type not
specified
-- http://phabricator.dev/oauthserver/auth/?client_id=X&response_type=code w/
existing authorization
--- got redirected to proper client url with pertinent code!
- tried a few important access calls
-- verified appropriate errors if missing any required parameters
-- verified good access code with appropriate other variables resulted in an
access token
- verified that if redirect_uri set correctly in authorization required for
access and errors if differs at all / only succeeds if exactly the same

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley, ajtrichards

Maniphest Tasks: T889, T906, T897

Differential Revision: https://secure.phabricator.com/D1727
2012-03-01 14:46:18 -08:00
Bob Trahan
3c4070a168 OAuth Server -- add controllers to RUD client authorizations and CRUD clients
Summary:
beyond the title, this diff tweaks the test console to have a bit more
functionality.  also makes a small change to CSS for AphrontFormControlMarkup,
which IMO fixes a display issue on
https://secure.phabricator.com/settings/page/profile/ where the Profile URI is
all up in the air and whatnot

I think this is missing pagination.  I am getting tired of the size though and
will add later.  See T905.

Test Plan:
viewed, updated and deleted client authorizations.  viewed, created,
updated and deleted clients

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T849, T850, T848

Differential Revision: https://secure.phabricator.com/D1683
2012-02-24 14:56:18 -08:00
Bob Trahan
af295e0b26 OAuth Server enhancements -- more complete access token response and groundwork
for scope

Summary:
this patch makes the access token response "complete" relative to spec by
returning when it expires AND that the token_type is in fact 'Bearer'.

This patch also lays the groundwork for scope by fixing the underlying data
model and adding the first scope checks for "offline_access" relative to expires
and the "whoami" method.   Further, conduit is augmented to open up individual
methods for access via OAuth generally to enable "whoami" access.   There's also
a tidy little scope class to keep track of all the various scopes we plan to
have as well as strings for display (T849 - work undone)

Somewhat of a hack but Conduit methods by default have SCOPE_NOT_ACCESSIBLE.  We
then don't even bother with the OAuth stuff within conduit if we're not supposed
to be accessing the method via Conduit.   Felt relatively clean to me in terms
of additional code complexity, etc.

Next up ends up being T848 (scope in OAuth) and T849 (let user's authorize
clients for specific scopes which kinds of needs T850).  There's also a bunch of
work that needs to be done to return the appropriate, well-formatted error
codes.  All in due time...!

Test Plan:
verified that an access_token with no scope doesn't let me see
anything anymore.  :(  verified that access_tokens made awhile ago expire.  :(

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, epriestley

Maniphest Tasks: T888, T848

Differential Revision: https://secure.phabricator.com/D1657
2012-02-21 16:33:06 -08:00
Bob Trahan
7a3f33b5c2 OAuth - Phabricator OAuth server and Phabricator client for new Phabricator OAuth Server
Summary:
adds a Phabricator OAuth server, which has three big commands:
 - auth - allows $user to authorize a given client or application.  if $user has already authorized, it hands an authoization code back to $redirect_uri
 - token - given a valid authorization code, this command returns an authorization token
 - whoami - Conduit.whoami, all nice and purdy relative to the oauth server.
Also has a "test" handler, which I used to create some test data.  T850 will
delete this as it adds the ability to create this data in the Phabricator
product.

This diff also adds the corresponding client in Phabricator for the Phabricator
OAuth Server.  (Note that clients are known as "providers" in the Phabricator
codebase but client makes more sense relative to the server nomenclature)

Also, related to make this work well
 - clean up the diagnostics page by variabilizing the provider-specific
information and extending the provider classes as appropriate.
 - augment Conduit.whoami for more full-featured OAuth support, at least where
the Phabricator client is concerned

What's missing here...   See T844, T848, T849, T850, and T852.

Test Plan:
- created a dummy client via the test handler.   setup development.conf to have
have proper variables for this dummy client.  went through authorization and
de-authorization flows
- viewed the diagnostics page for all known oauth providers and saw
provider-specific debugging information

Reviewers: epriestley

CC: aran, epriestley

Maniphest Tasks: T44, T797

Differential Revision: https://secure.phabricator.com/D1595
2012-02-19 14:00:13 -08:00