Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

use uuid instead of username for Bitbucket #1945

Closed
chadwhitacre opened this issue Jan 27, 2014 · 68 comments · Fixed by #2917
Closed

use uuid instead of username for Bitbucket #1945

chadwhitacre opened this issue Jan 27, 2014 · 68 comments · Fixed by #2917

Comments

@chadwhitacre
Copy link
Contributor

Update: Bitbucket recently rolled out uuids for their user API, so we can switch to that instead of dropping them. See below.


In the long run we're considering dropping all authentication via OAuth and only allowing password authentication (#1052). In the short run we need to drop Bitbucket as an auth method, because Bitbucket gives us no immutable user id, so if someone deletes their Bitbucket account, an attacker could create a Bitbucket account with their username and take over their Gittip account. @bruceadams demonstrated this here: #1807 (comment).


Want to back this issue? Place a bounty on it! We accept bounties via Bountysource.

@bruceadams
Copy link
Contributor

Agreed. I will work on collecting data about how many Gittip user's will be impacted and in what ways.

@zbynekwinkler
Copy link
Contributor

Any idea about how to approach this? We have in general no way of
contacting the affected users. Disabling bitbucket just for new users will
leave our UI in a strange state because we would have to show the UI to
sign in to all users and detect and differentiate opting in and say "sorry,
not for new users". It would also leave the current users in a potentially
vulnerable state. Disabling it for everyone would leave some users unable
to sign in.

@bruceadams
Copy link
Contributor

Bitbucket asks users for their email addresses. I don't know if we can get those addresses. I want data on how bad the problem is. How many users are only setup with Bitbucket? Of those, how many have tips? how many have a balance? If the list of people is small, we can try to track those people down one at a time. If the list is long, we may have to take a batch approach, possibly kicking out users 😦.

@bruceadams
Copy link
Contributor

Some basic Gittip numbers for BitBucket OAuth.

Count Description
3031 Total Gittip accounts with BitBucket OAuth setup
1181 BitBucket only
33 the related BitBucket user does not exist

@seanlinsley
Copy link
Contributor

What about active users? (either giving or receiving)

@bruceadams
Copy link
Contributor

I haven't gotten to active users. Of the 33 with OAuth tied to a nonexistent BitBucket user, 11 have no other OAuth set and 22 do have another OAuth setup. I'm going just blow away those 22 ties, since that appears to be a safe thing to do. (Once I've cleared the links, I'll publish that list here.)

@bruceadams
Copy link
Contributor

I just disconnected these 22 ties to BitBucket accounts that do not exist:

elsewhere id BitBucket user Gittip user
129618 aeloi joshschmelzle
117714 Apathetic Apathetic012
71195 beresovskiy beresovskiy
198619 davidhurley dbhurley
112860 duke_ dukex
102587 iekechukwu kwuchu
216689 jarobins jarobins
84431 jksk jespr
289655 jolaq djolaq
315458 jonahoffline jonahoffline
126040 kylescottmcgill kylescottmcgill
268658 MaddinXx MaddinXx
125868 montyman100 montyjanderson
325438 mortred kerimdzhanov
172766 nkistner wyze
126198 pooh110andco ale110
175176 rnh16 rhamzeh
126932 ronnyspringer ronny-springer
71780 sikaondrej sikaondrej
277196 theylive theylive
121852 tunixman computionist
110380 wanbokchoi wanbok

@zbynekwinkler
Copy link
Contributor

1181 does not seem like a number we can contact by hand 😦.
@bruceadams Were you able to find out if we have an access to the email on
Bitbucket?

@bruceadams
Copy link
Contributor

The Bitbucket API does not give out an email, but then, it doesn't give out account creation date either, when their web site does. I haven't dug around on their web site to see if I might be able to find email addresses there. Git commits will have email addresses, but it may not always be clear which commit (and email addresses) are associated with an account owner and some accounts may not have any commits.

@bruceadams
Copy link
Contributor

The eleven Gittip users which only have OAuth to a nonexistent Bitbucket user have never had incoming tips nor outgoing tips and no Stripe or Balanced financial tie setup. So, these users present no risk, they are only messy.

@zbynekwinkler
Copy link
Contributor

There is a possibility to "Send message" once you are logged in the
bitbucket website. No email to be seen anywhere. It seems it should be
possible to script this using requests http://docs.python-requests.org/.
Any better ideas? We can also show depreciation message to all users with
bitbucket only while on the gittip website.

I also am wondering if we just should drop the whole bitbucket
implementation. What would it mean when an bitbucket account is associated
to a gittip account but the bitbucket account is deleted? Or when someone
creates a new account with the same name? We are not in the "stealing
money" situation anymore but still, we have "this is confusing, not
trustful" situation which is also bad.

On 28 January 2014 11:48, Bruce Adams [email protected] wrote:

The eleven Gittip users which only have OAuth to a nonexistent Bitbucket
user have never had incoming tips nor outgoing tips and no Stripe or
Balanced financial tie setup. So, these users present no risk, they are
only messy.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1945#issuecomment-33467936
.

@zbynekwinkler
Copy link
Contributor

@bruceadams What are your thoughts on the way forward with this? I am willing to try the "send message" thing to notify bitbucket users (to attach another account). The next thing would be #900.

BTW: Bitbucket allows renames (nkistner is now most likely wyze but we have now way of confirming).

@seanlinsley
Copy link
Contributor

Everyone's periodic reminder that Bitbucket auth is unfixably broken, and that we need to remove it.

@chadwhitacre
Copy link
Contributor Author

Top.

@chadwhitacre
Copy link
Contributor Author

I've added the security label.

@chadwhitacre
Copy link
Contributor Author

This has been open long enough that we need to go through the exercise again.

@chadwhitacre
Copy link
Contributor Author

Some basic Gratipay numbers for Bitbucket OAuth.

Count Description SQL
4319 Total Gratipay accounts with Bitbucket OAuth setup select count(*) from elsewhere where platform='bitbucket';
2799 Not Bitbucket only select count(*) from (select e1.user_name from elsewhere e1 join elsewhere e2 on e1.participant=e2.participant where e1.platform='bitbucket' and e2.platform in ('twitter', 'github', 'facebook', 'google', 'openstreetmap') group by e1.user_name) as foo;
1520 Bitbucket only select 4319 - 2799;
? the related Bitbucket user does not exist

@chadwhitacre
Copy link
Contributor Author

We're going to have to drop some users entirely. Anyone with Bitbucket as their only signin method will no longer have a Gratipay account.

We will drop some people who are giving through Gratipay.

We will drop some people who are receiving through Gratipay.

The real hard part will be dropping people who have a balance on Gratipay.

We should try our best to notify people who are giving, receiving, and/or have a balance on Gratipay. We can do this by trying to find emails for people, and then beyond that we'll just have to depend on a blog post about it.

@chadwhitacre
Copy link
Contributor Author

Question: Are we going to keep Bitbucket as a non-sign-in auth method? I say no. With no immutable ID there's no question but that those connections are going to degrade over time as people change their Bitbucket usernames. I say we drop Bitbucket entirely.

@chadwhitacre
Copy link
Contributor Author

So how many people in each category are we going to have to drop?

Category Number
only giving 14
only receiving 1
giving and receiving 0
has a balance 28

@chadwhitacre
Copy link
Contributor Author

We have 39 users that are either giving or receiving or have a balance, and can only sign in using Bitbucket.

     select e1.platform, e1.user_name, p.username, p.balance, p.receiving, p.giving                     
       from elsewhere e1                                                                                
  left join elsewhere e2                                                                                
         on e1.participant = e2.participant                                                             
        and e1.platform = 'bitbucket'                                                                   
        and e2.platform in ('twitter', 'github', 'facebook', 'google', 'openstreetmap')                 
       join participants p                                                                              
         on e1.participant = p.username                                                                 

      where e1.platform = 'bitbucket'                                                                   
        and e2.platform is null                                                                         
        and (p.balance + p.receiving + p.giving) > 0                                                    

   order by p.balance desc                                                                              
           ;

@chadwhitacre
Copy link
Contributor Author

36 if we exclude suspicious accounts.

drop table if exists only_bitbucket;
create temp table only_bitbucket as (

     select e1.platform, e1.user_name, p.username, p.balance, p.receiving, p.giving
       from elsewhere e1
  left join elsewhere e2
         on e1.participant = e2.participant
        and e1.platform = 'bitbucket'
        and e2.platform in ('twitter', 'github', 'facebook', 'google', 'openstreetmap')
       join participants p
         on e1.participant = p.username
        and p.is_suspicious is not true

      where e1.platform = 'bitbucket'
        and e2.platform is null
        and (p.balance + p.receiving + p.giving) > 0

   order by p.balance desc
);
select * from only_bitbucket;

@chadwhitacre
Copy link
Contributor Author

30 if we exclude unclaimed accounts:

drop table if exists only_bitbucket;
create temp table only_bitbucket as (
     select e1.platform, e1.user_name, p.username, p.balance, p.receiving, p.giving
       from elsewhere e1
  left join elsewhere e2
         on e1.participant = e2.participant
        and e1.platform = 'bitbucket'
        and e2.platform in ('twitter', 'github', 'facebook', 'google', 'openstreetmap')
       join participants p
         on e1.participant = p.username
        and p.is_suspicious is not true
        and p.claimed_time is not null

      where e1.platform = 'bitbucket'
        and e2.platform is null
        and (p.balance + p.receiving + p.giving) > 0

   order by p.balance desc
);
select * from only_bitbucket;

@chadwhitacre
Copy link
Contributor Author

┌─────────┐
│ balance │
├─────────┤
│   30.00 │
│    9.16 │
│    9.05 │
│    8.82 │
│    8.64 │
│    7.87 │
│    7.23 │
│    5.73 │
│    4.82 │
│    4.55 │
│    4.44 │
│    4.05 │
│    3.60 │
│    2.45 │
│    1.81 │
│    1.71 │
│    1.64 │
│    1.64 │
│    1.51 │
│    1.46 │
│    0.82 │
│    0.82 │
│    0.41 │
│    0.32 │
│    0.23 │
│    0.21 │
│    0.20 │
│    0.16 │
└─────────┘
┌─────────┐
│   sum   │
├─────────┤
│  123.35 │
└─────────┘

@chadwhitacre
Copy link
Contributor Author

I searched for uuid on https://confluence.atlassian.com/ using both Google and site search, and found no results.

@erikvanzijst
Copy link
Contributor

Indeed, we added that a few weeks ago, but our doco is lagging behind.

That is a public field that you should use. It's on repos too (though you wouldn't care as much about that I'd think).

You can substitute usernames and uuids in URLs. These URLs are equivalent:

https://api.bitbucket.org/2.0/users/evzijst
https://api.bitbucket.org/2.0/users/{a288a0ab-e13b-43f0-a689-c4ef0a249875}

@erikvanzijst
Copy link
Contributor

The curly braces let us distinguish between a UUID string and a username that looks like one. Curly braces are illegal in usernames (you will have to urlencode them, which I omitted in my previous comment for clarity).

@chadwhitacre
Copy link
Contributor Author

Awesome, thanks @erikvanzijst! Congrats on rolling out uuids, we'll pursue that route. :-)

@chadwhitacre
Copy link
Contributor Author

Here's a little script to take a list of usernames on stdin and spit out {200,404} username on stdout. It's useful to get an idea of how often people change their Bitbucket usernames, as @bruceadams did above.

import requests, sys

for line in sys.stdin:
    username = line.strip()
    response = requests.get('https://bitbucket.org/api/2.0/users/' + username)
    print response.status_code, username
    sys.stdout.flush()

@chadwhitacre
Copy link
Contributor Author

We have 4,278 non-team Bitbucket accounts on file. Of these 4,211 are 200 and 67 (1.6%) are 404, indicating a changed username. We can't tell (of course) how many of the 200s are accounts that have changed uuids since we filed them.

@chadwhitacre
Copy link
Contributor Author

Okay! Here's how I suggest we proceed:

  1. Update our Bitbucket integration to key off of uuid instead of username for new Bitbucket elsewheres (natch).
  2. Drop Bitbucket for the 2,808 accounts that also have another elsewhere attached. We have no guarantee that the Bitbucket account is indeed theirs. The risk of a false positive is high (there's $21,749.45 in 569 affected accounts), and the risk of a false negative is low (they can easily reconnect Bitbucket). [Note: make sure the other account elsewhere is not is_team!]
  3. For the 1,521 accounts that are only linked to Bitbucket, I propose that we link it to whatever Bitbucket uuid is currently associated with the Bitbucket username we have on file. If a given such account doesn't have a balance/giving/receiving, then it doesn't matter whether someone has compromised their account, because their Gratipay account is now functionally equivalent to the account that any attacker could have created using the Bitbucket username that had been let go. There are only 36 non-suspicious accounts that do have a balance/giving/receiving (6 are unclaimed). The chance of a false positive is low: 1.6% of 36 is roughly 0. The risk associated with a false positive is also low: there's only $123.35 in balance and $3.40/wk in receiving across these accounts. On the other hand, forcibly closing these accounts is both difficult (implement forcible account closing #2909), and heavy-handed. We should make a good-faith effort beyond just a blog post to notify the 36 affected users.

@chadwhitacre
Copy link
Contributor Author

then it doesn't matter whether someone has compromised their account

We should also look at whether a Bitbucket-connected-account has a funding instrument attached (credit card, bank account, PayPal, bitcoin).

@chadwhitacre
Copy link
Contributor Author

Of the 1,521 Bitbucket-only Gratipay accounts, there are five with bitcoin addresses on file, one with PayPal, two with bank accounts (both working), and 34 with credit cards (24 working). That's 42 accounts total (there's no overlap amongst those four categories).

@chadwhitacre
Copy link
Contributor Author

Nine of the 1,521 have already been closed by the account owner.

@chadwhitacre
Copy link
Contributor Author

Should we put some sort of notice under "Connected Accounts" with a link to our blog post about this? It would only be relevant for people who had another account connected.

@chadwhitacre chadwhitacre changed the title drop Bitbucket as auth method use uuid instead of username for Bitbucket Nov 10, 2014
@techtonik
Copy link
Contributor

It is probably lazyness that speaks over me that it is probably not worth
to annoy people with these details. But it may be possible to implement
some kind of universal mechanism for one-time actions or notifications. If
I am not mistaken, something like Redis bitmaps is an ideal tool for this
kind of problem.

@chadwhitacre
Copy link
Contributor Author

Should we put some sort of notice under "Connected Accounts" with a link to our blog post about this?

More trouble than it's worth.

@chadwhitacre
Copy link
Contributor Author

I.e., +1 @techtonik. :-)

@chadwhitacre
Copy link
Contributor Author

I'm thinking of doing the user_id, is_team update for all Bitbucket accounts, regardless of whether or not we're going to subsequently delete some portion of them.

@chadwhitacre
Copy link
Contributor Author

Now for a blog post.

@chadwhitacre
Copy link
Contributor Author

@chadwhitacre
Copy link
Contributor Author

Post revised. I took out the update re: Facebook and Google+. We'll do that in a separate post. This is significant enough to warrant its own post.

@chadwhitacre
Copy link
Contributor Author

We have 4,278 non-team Bitbucket accounts on file. Of these 4,211 are 200 and 67 (1.6%) are 404, indicating a changed username. We can't tell (of course) how many of the 200s are accounts that have changed uuids since we filed them.

Some of those 404s turned out to be team/user switches. The total number of affected accounts is 4,332, and the number of true 404s is 24, and 24 / 4,332 is 0.554%.

@chadwhitacre
Copy link
Contributor Author

We only have three email addresses on file for the 50 non-suspicious accounts that are Bitbucket-only and have giving/receiving/balance or a funding instrument.

@chadwhitacre
Copy link
Contributor Author

Used this for some figures in the blog post:

drop table if exists only_bitbucket;
create temp table only_bitbucket as (
     select p.username, e1.user_name, p.email
          , p.claimed_time, p.is_suspicious, p.is_closed
          , p.balance, p.receiving, p.giving
          , p.last_ach_result, p.last_bill_result, p.paypal_email, p.bitcoin_address
       from elsewhere e1
  left join elsewhere e2
         on e1.participant = e2.participant
        and e1.platform = 'bitbucket'
        and e2.platform in ('twitter', 'github', 'facebook', 'google', 'openstreetmap')
       join participants p
         on e1.participant = p.username

      where e1.platform = 'bitbucket'
        and e2.platform is null

   order by p.balance desc
);
select count(*) from only_bitbucket;
select count(*), sum(balance), sum(giving+receiving) from only_bitbucket
 where ((giving + receiving + balance > 0)
        or last_ach_result is not null
        or last_bill_result is not null
        or paypal_email is not null
        or bitcoin_address is not null)
       and (is_suspicious is not true)
;

drop table if exists not_only_bitbucket;
create temp table not_only_bitbucket as (
     select p.username, e.user_name
          , p.claimed_time, p.is_suspicious, p.is_closed
          , p.balance, p.receiving, p.giving
          , p.last_ach_result, p.last_bill_result, p.paypal_email, p.bitcoin_address
       from participants p
       join elsewhere e
         on p.username = e.participant
            and e.platform='bitbucket'
            and e.user_name in (
                 select e1.user_name
                   from elsewhere e1
                   join elsewhere e2
                     on e1.participant=e2.participant
                        and e1.platform='bitbucket'
                        and e2.platform in ('twitter', 'github', 'facebook', 'google', 'openstreetmap')
               group by e1.user_name
            )
);

select count(*) from not_only_bitbucket;
select count(*), sum(balance), sum(giving+receiving) from not_only_bitbucket
 where ((giving + receiving + balance > 0)
        or last_ach_result is not null
        or last_bill_result is not null
        or paypal_email is not null
        or bitcoin_address is not null)
       and (is_suspicious is not true)
;

@chadwhitacre
Copy link
Contributor Author

None of the 24 true orphans have any giving/receiving/balance.

select p.username, e.user_name, p.giving, p.receiving, p.balance, p.last_ach_result, p.last_bill_result, p.paypal_email, p.bitcoin_address
from elsewhere e join participants p on e.participant=p.username 
where platform='bitbucket' and user_id not like '{%';

@chadwhitacre
Copy link
Contributor Author

IRC re: blog post.

@erikvanzijst
Copy link
Contributor

@whit537 I should mention another API that is still awaiting documentation.

I'm assuming that to obtain the Bitbucket username after the OAuth dance, you hit https://api.bitbucket.org/1.0/user and look at the username and then hit https://api.bitbucket.org/2.0/users/evzijst to pull out the uuid.

Aside from the fact that that /1.0/user API returns a ton of mostly unsolicited repo data that makes it very slow for users with a ton of stuff, it's also a 2-step process.

There's a 2.0 version of the /user API now that scales a lot better. Also, it contains the uuid in the response, which will likely allow you cut out an API call:

https://bitbucket.org/api/2.0/user

@chadwhitacre
Copy link
Contributor Author

@erikvanzijst Thanks, yes! We're using the 2.0 user endpoint. :-)

@erikvanzijst
Copy link
Contributor

Sweet!

P.S.
Instead of /2.0/teams/{user_name}/members you can use /2.0/teams/{uuid}/members on line

api_team_members_path = '/2.0/teams/{user_name}/members'

Terribly minor, but that the former has the tiniest race condition that the uuid does not have.

@chadwhitacre
Copy link
Contributor Author

@erikvanzijst Cool, thanks. I think we're constrained by our elsewhere implementation as to what is available for interpolation in that context, but I'll keep that in mind next time I end up hacking on that code. :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants