1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-05 05:02:44 +01:00
Commit graph

107 commits

Author SHA1 Message Date
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
vrana
5bde6c71ce Declare used properties
Summary: Also fix couple of other errors.

Test Plan: Executed controllers.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3627
2012-10-04 14:46:06 -07:00
epriestley
6b1c27eb0e Make "public" pastes meaningfully visible to logged-out users
Summary:
  - Introduce `shouldAllowPublic()`, indicating that logged-out users are OK in a controller if the install is configured to permit public policies.
  - Make Paste views and lists allow public users.
  - Make UI do sensible things with respect to disabling links, etc.
  - Improve behavior of "you need to login" with respect to policy exceptions and Ajax requests.

Test Plan: Looked at "public" paste, saw all unavailable UI disabled, clicked it, got appropraite prompts.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T603

Differential Revision: https://secure.phabricator.com/D3502
2012-09-30 19:44:09 -07:00
epriestley
2d66798c5d Fix one more issue for username "0". 2012-09-13 17:05:53 -07:00
epriestley
0e672e2435 Fix GitHub login failure for user with username "0". 2012-09-13 16:47:14 -07:00
epriestley
e3c6dc687a Fall back to LDAP search attribute if username attribute is not configured
Summary:
See discussion in D3340. Some configurations set only a search attribute because their records are indexed by username (this is probably not quite the correct LDAP terminology). Other configurations use one attribute to search and a different attribute to select usernames.

After D3340, installs which set only a search attribute broke. Instead, fall back to the search attribute if no username attribute is present.

Test Plan: Successfully logged in on my test slapd.

Reviewers: yunake, voldern, briancline

Reviewed By: voldern

CC: aran

Differential Revision: https://secure.phabricator.com/D3406
2012-09-03 06:41:49 -07:00
Brian Cline
b0802a7797 Resolve LDAP registration DAO exception due to empty username
Summary:
When logging in as an LDAP user for the first time (thus registering), a DAO exception was being thrown because PhabricatorLDAPRegistrationController wasn't passing in a username to PhabricatorUser::setUsername().

Somewhat separately, since either the PHP LDAP extension's underlying library or Active Directory are returning attributes with lowercased key names, I have to search on sAMAccountName and look for the key samaccountname in the results; this is fine since the config allows these to be defined separately. However I found that PhabricatorLDAPProvider::retrieveUserName() was attempting to use the search attribute rather than the username attribute. This resolves.

Test Plan: Tested registration and login against our internal AD infrastructure; worked perfectly. Need help from someone with access to a functional non-AD LDAP implementation; I've added the original author and CCs from D2722 in case they can help test in this regard.

Reviewers: epriestley, voldern

Reviewed By: voldern

CC: voldern, aran, Korvin, auduny, svemir

Differential Revision: https://secure.phabricator.com/D3340
2012-08-24 08:43:02 -07:00
epriestley
d5aaa54d0b Allow installs to configure an arbitrary block of HTML to show on the login screen
Summary: For secure.phabricator.com, I've wanted to put up a "you can use demo/demo" message for a while. Wikimedia also wants to add some custom login instructions.

Test Plan:
Configured this:

    'auth.login-message' =>
      '<div style="width: 50%; margin: 1em auto; padding: 1em; border: 1px solid green; background: #dfffdf;">'.
        '<strong>Welcome!</strong> Lorem ipsum...'.
      '</div>',

...got this:

{F17167}

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran, jeremyb

Maniphest Tasks: T1637

Differential Revision: https://secure.phabricator.com/D3288
2012-08-14 19:11:46 -07:00
epriestley
47b96fdebe Use sprites for all the menu icons
Summary:
See D3277, D3278.

  - Sprite all the menu icons.
  - Delete the unsprited versions.
  - Notification bolt now uses the same style as everything else.

Test Plan: Looked at page, hovered, clicked things.

Reviewers: btrahan, chad, vrana

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1569

Differential Revision: https://secure.phabricator.com/D3279
2012-08-14 14:20:01 -07:00
Bob Trahan
f748a47237 strncmp is better than strpos
Summary: no need to get all O(N) up in this when we can do constant time of "8"

Test Plan: arc lint

Reviewers: vrana, epriestley

Reviewed By: vrana

CC: aran, Korvin

Maniphest Tasks: T891

Differential Revision: https://secure.phabricator.com/D3271
2012-08-13 18:02:28 -07:00
Bob Trahan
7bb3c39cde Polish removal of conduit shield, including legacy stripping for phabricator on phabricator oauth scenarios
Summary: ...just in case that stuff happens in the "wild". also cleaned up the logic here since we no longer have the conduit conditionality.

Test Plan: made sure I didn't break JS on the site. reasoned about logic of my function and asking people PHP typing questions in job interviews.

Reviewers: epriestley, vrana

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T891

Differential Revision: https://secure.phabricator.com/D3269
2012-08-13 16:05:56 -07:00
Bob Trahan
dd26bc6d1a Remove shield for Conduit API responses
Summary: 'cuz we don't need it and it's lame complexity for API clients of all kinds. Rip the band-aid off now.

Test Plan: used conduit console and verified no more shield. also did some JS stuff around the suite to verify I didn't kill JS

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T891

Differential Revision: https://secure.phabricator.com/D3265
2012-08-13 14:49:32 -07:00
epriestley
20ac900e8b Make settings panels more modular and modern
Summary:
Currently, we have a hard-coded list of settings panels. Make them a bit more modular.

  - Allow new settings panels to be defined by third-party code (see {D2340}, for example -- @ptarjan).
  - This makes the OAuth stuff more flexible for {T887} / {T1536}.
  - Reduce the number of hard-coded URIs in various places.

Test Plan: Viewed / edited every option in every panel. Grepped for all references to these URIs.

Reviewers: btrahan, vrana, ptarjan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3257
2012-08-13 12:37:26 -07:00
epriestley
81f8b507fe Use shinier menu icons
Summary:
  - Use @chad's nice gradient overlay icons.
  - Show selected states.
  - Use profile picture for profile item (not sure about this treatment?)
  - Workflow the logout link

Test Plan: Will add screenshots.

Reviewers: alanh, btrahan, chad

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3225
2012-08-10 12:11:24 -07:00
epriestley
ebb7807bb4 Fix an issue with URIs missing trailing slashes, like "/maniphest"
Summary:
In D3144, I made us look in application maps to find routing rules. However, we don't process //all// the maps when we 404 and try to do a "/" redirect. Process all of the maps.

Additionally, in D3146 I made the menu items application-driven. However, some pages (like 404) don't have a controller. Drop the requirement that the controller be nonnull.

Test Plan:
  - Visited "/maniphest", got a redirect after this patch.
  - Visited "/asldknfalksfn", got a 404 after this patch.

Reviewers: davidreuss, vrana, btrahan

Reviewed By: davidreuss

CC: aran

Maniphest Tasks: T1607

Differential Revision: https://secure.phabricator.com/D3158
2012-08-06 12:46:51 -07:00
Danny Su
92fd606df3 Allow custom LDAP port
Summary: Allow custom LDAP port to be defined in config file

Test Plan: Test login works by specifying a custom port

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3153
2012-08-05 15:37:40 -07:00
epriestley
314c25365d Drive menu icons from Applications
Summary:
This makes no changes, it just moves the menu icons to the applications instead of hard-coded on the page.

I'm going to try to address some of the angst in T1593 next...

Test Plan: Loaded logged-in / logged out pages. Clicked menu items. Looked at /applications/.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1593, T1569

Differential Revision: https://secure.phabricator.com/D3146
2012-08-05 14:12:43 -07:00
Evan Priestley
94445d41e7 Merge pull request #168 from ossareh/support_ldap_search_for_users
Support ldap search for users
2012-07-26 14:54:10 -07:00
Michael Ossareh
a9af5d611d Prevent the ability to scrape for valid usernames
- return the same error message when either bind or the username search
   fails to find a user
 - config variables should use hypen and not underscore
2012-07-26 14:32:51 -07:00
Michael Ossareh
0a0607d2f7 Support searching for users to find their LDAP entry
Summary:
 - the current LDAP auth flow expects a DN to look like
   cn=ossareh,ou=Users,dc=example,dc=com
 - however many LDAP setups have their dn look something like
   cn=Mike Ossareh,ou=Users,dc=example,dc=com

Test Plan:
 Test if logins work with a LDAP setup which has cn=Full Name
 instead of cn=username.

 To test you should ensure you set the properties needed to
 trigger the search before login as detailed in conf/default.conf.php

Reviewers: epriestley

CC: mbeck, aran, Korvin

Differential Revision: https://secure.phabricator.com/D3072
2012-07-25 19:03:10 -07:00
epriestley
94bc4003f1 Show a "Logout?" dialog when the user goes to /logout/
Summary: Currently, the logout link is this awkward form in the footer since we have to CSRF it. Enable a GET + dialog + confirm workflow instead so the logout link can just be a link instead of this weird mess.

Test Plan: Went to /logout/, logged out.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D3066
2012-07-25 13:58:49 -07:00
Marcel Beck
823555ce30 Add LDAP Referrals Option
Summary: In order to perform the searches on Windows 2003 Server Active Directory you have to set the LDAP_OPT_REFERRALS option to 0

Test Plan: Test if LDAP works with Windows 2003 AD

Reviewers: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3004
2012-07-18 23:29:40 +02:00
Neal Poole
7081923bd7 Adding 'Secure Browsing' config setting to Facebook OAuth.
Summary: The Graph API exposes a new field, security_settings, which allows applications to see whether a user has enabled Secure Browsing. This diff adds a configuration setting to Phabricator which forces users to have Secure Browsing enabled when logging in via Facebook.

Test Plan: With the configuration setting off, verify that secure browsing does not affect the ability to log in. With the configuration setting on and secure browsing off, verify that the login attempts is rejected. Then verify that the login attempt succeeds when secure browsing is enabled.

Reviewers: epriestley

Reviewed By: epriestley

CC: arice, aran, Korvin

Maniphest Tasks: T1487

Differential Revision: https://secure.phabricator.com/D2964
2012-07-17 18:18:16 -07:00
epriestley
30deacdbaf Fix some more ldap issues
Summary:
  - LDAP import needs to use envelopes.
  - Use ldap_sprintf().

Test Plan: Configured an LDAP server. Added an account. Imported it; logged in with it. Tried to login with accounts like ",", etc., got good errors.

Reviewers: vrana, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D2995
2012-07-17 14:05:26 -07:00
epriestley
dd70c59465 Use OpaqueEnvelopes for all passwords in Phabricator
Summary:
See D2991 / T1526. Two major changes here:

  - PHP just straight-up logs passwords on ldap_bind() failures. Suppress that with "@" and keep them out of DarkConsole by enabling discard mode.
  - Use PhutilOpaqueEnvelope whenever we send a password into a call stack.

Test Plan:
  - Created a new account.
  - Reset password.
  - Changed password.
  - Logged in with valid password.
  - Tried to login with bad password.
  - Changed password via accountadmin.
  - Hit various LDAP errors and made sure nothing appears in the logs.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2993
2012-07-17 12:06:33 -07:00
Avishay Lavie
226cf288e9 Add active-directory domain-based ldap authentication support
Summary: Add active-directory domain-based ldap authentication support

Test Plan: Tested on a live install against Active Directory on a Windows Server

Reviewers: epriestley

CC: aran, epriestley

Maniphest Tasks: T1496

Differential Revision: https://secure.phabricator.com/D2966
2012-07-13 15:19:34 +03:00
Evan Priestley
62e039b748 Merge pull request #150 from linead/ldap_dir
Ldap dir
2012-07-11 18:37:36 -07:00
linead
51d2d06e37 Added ldap import controller 2012-07-04 12:10:38 +10:00
vrana
2b946459b5 Inform user about associated accounts in failed login
Summary:
Sometimes people create their account in Phabricator using some OAuth provider, forget about it and then tries to login with another provider.

Provide this information to them.

Test Plan: Tried to link an account linked to already existing user.

Reviewers: epriestley

Reviewed By: epriestley

CC: robarnold, aran, Korvin

Differential Revision: https://secure.phabricator.com/D2857
2012-06-26 11:38:07 -07:00
Daniel Fullarton
37df05c7a5 T1354 : Resize profile pics (specifically to handle github v3 api)
Reviewed by: epriestley
GitHub Pull: https://github.com/facebook/phabricator/pull/140
2012-06-25 06:29:19 -07:00
linead
e683236793 Extract ldap auth from password auth completely 2012-06-24 18:12:56 -07:00
vrana
c762050b7c Get rid of file_get_contents($uri)
Summary: It requires `allow_url_fopen` which we don't check in setup and our installation is about to disable it.

Test Plan:
Login with OAuth.
/oauth/facebook/diagnose/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D2787
2012-06-18 17:45:45 -07:00
Espen Volden
726041584f Made it possible to login using LDAP
Summary: Made it possible to link and unlink LDAP accounts with  Phabricator accounts.

Test Plan:
I've tested this code locally and in production where I work.
I've tried creating an account from scratch by logging in with LDAP and linking and unlinking an LDAP account with an existing account. I've tried to associate the same LDAP account with different Phabricator accounts and it failed as expected.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin, auduny, svemir

Maniphest Tasks: T742

Differential Revision: https://secure.phabricator.com/D2722
2012-06-13 08:58:46 -07:00
epriestley
06371bc2cd Upgrade to GitHub v3 API
Summary: GitHub dropped support for the v2 API today, which breaks login and registration. Use the v3 API instead.

Test Plan: Registered and logged in with GitHub. Verified process pulled email/photo/name/username correctly.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D2726
2012-06-12 12:13:27 -07:00
epriestley
0e1bbbd489 Allow administrators to change usernames
Summary:
Give them a big essay about how it's dangerous, but allow them to do it formally.

Because the username is part of the password salt, users must change their passwords after a username change.

Make password reset links work for already-logged-in-users since there's no reason not to (if you have a reset link, you can log out and use it) and it's much less confusing if you get this email and are already logged in.

Depends on: D2651

Test Plan: Changed a user's username to all kinds of crazy things. Clicked reset links in email. Tried to make invalid/nonsense name changes.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1303

Differential Revision: https://secure.phabricator.com/D2657
2012-06-06 07:09:56 -07:00
epriestley
0a7b4591ef Allow usernames to include ".", "-" and "_"
Summary:
See T1303, which presents a reasonable case for inclusion of these characters in valid usernames.

Also, unify username validity handling.

Test Plan: Created a new user with a valid name. Tried to create a new user with an invalid name. Ran unit tests.

Reviewers: btrahan, vrana

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1303

Differential Revision: https://secure.phabricator.com/D2651
2012-06-06 07:09:05 -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
vrana
1ebf9186b4 Depend on class autoloading
Test Plan:
Run setup.
/differential/

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1103

Differential Revision: https://secure.phabricator.com/D2612
2012-05-30 16:57:21 -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
epriestley
557e508656 Allow restriction of permitted email domains
Summary:
Allow allowed email addresses to be restricted to certain domains. This implies email must be verified.

This probably isn't QUITE ready for prime-time without a few other tweaks (better administrative tools, notably) but we're nearly there.

Test Plan:
  - With no restrictions:
    - Registered with OAuth
    - Created an account with accountadmin
    - Added an email
  - With restrictions:
    - Tried to OAuth register with a restricted address, was prompted to provide a valid one.
    - Tried to OAuth register with a valid address, worked fine.
    - Tried to accountadmin a restricted address, got blocked.
    - Tried to accountadmin a valid address, worked fine.
    - Tried to add a restricted address, blocked.
    - Tried to add a valid address, worked fine.
    - Created a user with People with an invalid address, got blocked.
    - Created a user with People with a valid address, worked fine.

Reviewers: btrahan, csilvers

Reviewed By: csilvers

CC: aran, joe, csilvers

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2581
2012-05-26 06:04:35 -07:00
epriestley
70fd96037b Consolidate user editing code
Summary:
  - We currently have some bugs in account creation due to nontransactional user/email editing.
    - We save $user, then try to save $email. This may fail for various reasons, commonly because the email isn't unique.
    - This leaves us with a $user with no email.
  - Also, logging of edits is somewhat inconsistent across various edit mechanisms.
  - Move all editing to a `PhabricatorUserEditor` class.
  - Handle some broken-data cases more gracefully.

Test Plan:
  - Created and edited a user with `accountadmin`.
  - Created a user with `add_user.php`
  - Created and edited a user with People editor.
  - Created a user with OAuth.
  - Edited user information via Settings.
  - Tried to create an OAuth user with a duplicate email address, got a proper error.
  - Tried to create a user via People with a duplicate email address, got a proper error.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: tberman, aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2569
2012-05-25 07:30:44 -07:00
epriestley
77f546c572 Allow installs to require email verification
Summary:
Allow installs to require users to verify email addresses before they can use Phabricator. If a user logs in without a verified email address, they're given instructions to verify their address.

This isn't too useful on its own since we don't actually have arbitrary email registration, but the next step is to allow installs to restrict email to only some domains (e.g., @mycompany.com).

Test Plan:
  - Verification
    - Set verification requirement to `true`.
    - Tried to use Phabricator with an unverified account, was told to verify.
    - Tried to use Conduit, was given a verification error.
    - Verified account, used Phabricator.
    - Unverified account, reset password, verified implicit verification, used Phabricator.
  - People Admin Interface
    - Viewed as admin. Clicked "Administrate User".
    - Viewed as non-admin
  - Sanity Checks
    - Used Conduit normally from web/CLI with a verified account.
    - Logged in/out.
    - Sent password reset email.
    - Created a new user.
    - Logged in with an unverified user but with the configuration set to off.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran, csilvers

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2520
2012-05-21 12:47:38 -07:00
epriestley
905cc5ff32 Minor, fix OAuth registration, see D2431.
Auditors: btrahan
2012-05-09 11:19:15 -07:00
Bob Trahan
679f778235 OAuth -- add support for Disqus
Summary:
also fix some bugs where we weren't properly capturing the expiry value or scope of access tokens.

This code isn't the cleanest as some providers don't confirm what scope you've been granted. In that case, assume the access token is of the minimum scope Phabricator requires. This seems more useful to me as only Phabricator at the moment really easily / consistently lets the user increase / decrease the granted scope so its basically always the correct assumption at the time we make it.

Test Plan: linked and unlinked Phabricator, Github, Disqus and Facebook accounts from Phabricator instaneces

Reviewers: epriestley

Reviewed By: epriestley

CC: zeeg, aran, Koolvin

Maniphest Tasks: T1110

Differential Revision: https://secure.phabricator.com/D2431
2012-05-08 12:08:05 -07:00
epriestley
87207b2f4e Allow users to have multiple email addresses, and verify emails
Summary:
  - Move email to a separate table.
  - Migrate existing email to new storage.
  - Allow users to add and remove email addresses.
  - Allow users to verify email addresses.
  - Allow users to change their primary email address.
  - Convert all the registration/reset/login code to understand these changes.
  - There are a few security considerations here but I think I've addressed them. Principally, it is important to never let a user acquire a verified email address they don't actually own. We ensure this by tightening the scoping of token generation rules to be (user, email) specific.
  - This should have essentially zero impact on Facebook, but may require some minor changes in the registration code -- I don't exactly remember how it is set up.

Not included here (next steps):

  - Allow configuration to restrict email to certain domains.
  - Allow configuration to require validated email.

Test Plan:
This is a fairly extensive, difficult-to-test change.

  - From "Email Addresses" interface:
    - Added new email (verified email verifications sent).
    - Changed primary email (verified old/new notificactions sent).
    - Resent verification emails (verified they sent).
    - Removed email.
    - Tried to add already-owned email.
  - Created new users with "accountadmin". Edited existing users with "accountadmin".
  - Created new users with "add_user.php".
  - Created new users with web interface.
  - Clicked welcome email link, verified it verified email.
  - Reset password.
  - Linked/unlinked oauth accounts.
  - Logged in with oauth account.
  - Logged in with email.
  - Registered with Oauth account.
  - Tried to register with OAuth account with duplicate email.
  - Verified errors for email verification with bad tokens, etc.

Reviewers: btrahan, vrana, jungejason

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1184

Differential Revision: https://secure.phabricator.com/D2393
2012-05-07 10:29:33 -07:00
epriestley
a122336b3e Try to diagnose App Login only for OAuth providers which support it
Summary: We currently try to do "app login" for all OAuth providers, but not all of them support it in a meaningful way. Particularly, it always fails for Google.

Test Plan: Ran google diagnostics on a working config, no longer got a diagnostic failure.

Reviewers: btrahan, vrana, csilvers

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2377
2012-05-04 16:16:29 -07:00
epriestley
6a04328430 Tighten scope requests with Google OAuth
Summary: We currently make a ludicrously gigantic permission request to do Google auth (read/write access to the entire address book), since I couldn't figure out how to do a more narrowly tailored request when I implemented it. @csilvers pointed me at some much more sensible APIs; we can now just ask for user ID, name, and email address.

Test Plan: Created a new account via Google Oauth. Linked/unlinked an existing account. Verified diagnostics page still works correctly. Logged in with a pre-existing Google account created with the old API (to verify user IDs are the same through both methods).

Reviewers: btrahan, vrana, csilvers, Makinde

Reviewed By: csilvers

CC: aran

Differential Revision: https://secure.phabricator.com/D2378
2012-05-03 13:25:06 -07:00
Bob Trahan
8988667dcc Make Oauth-registration flows a bit more resilient to failures from the providers
Summary: basically by validating we have good user data when we set the user data.

Test Plan: simulated a failure from a phabricator on phabricator oauth scenario. viewed ui that correctly told me there was an error with the provider and to try again.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1077

Differential Revision: https://secure.phabricator.com/D2337
2012-05-01 11:51:40 +02:00
Bob Trahan
8069694640 Fix next_uri
Summary: we were using the "path" as the next_uri and that drops some delicious get parameters

Test Plan: see T1140; basically re-ran the steps listed there and they passed!

Reviewers: epriestley, njhartwell

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T1140, T1009

Differential Revision: https://secure.phabricator.com/D2299
2012-04-22 20:39:54 +02:00
vrana
d9ce80aa17 Explicitly set URL parameter separator for external URLs
Summary:
PHP has this crazy [[ http://php.net/arg_separator.output | arg_separator.output ]] INI setting which allows setting different string for URL parameters separator instead of `&` (e.g. in `?a=1&b=2`).

Don't use it for external URLs.

Test Plan: Log in through OAuth.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2284
2012-04-19 14:54:47 -07:00