#metabrainz

is the new oauth provider url supposed to give Page not found? :)

my bad, correct link is https://test.musicbrainz.org/new-oauth2/client/...

should we start?

sure, let's start

Sure

1. Current Status of OAuth Provider

so the provider is more or less ready for use, its deployed on test.mb and i also modified CB's codebase to test both login and using the access tokens issued by this provider to write reviews using its API.

some UI issues need to be fixed, i'll discuss them separately with monkey and aerozol.

!m lucifer

You're doing good work, lucifer!

nothing much to discuss about this point so moving on to 2.

2. Security measures taken and decided against

the distrinet group had done a audit of the MB OAuth provider a few years ago, using their now open source tool. i audited the new ouath provder's implementation.

The report is ^. There are no major unmitigated threats according to it.

However, there are some tradeoffs that can be made to make it even more secure.

there are a total of 8 failing tests in the report. 3 are mandatory, i have opened PRs upstream to fix them in authlib (the library used to implement this provider).

i do not think these pose a security threat though so it should be fine to release even without it.

does anyone think otherwise?

the MB revoke endpoint requires a client_id and client_secret, is that not is what is meant by "Is revocation bound to a specific client"?

right the current oauth provider requires that as well. but authlib doesn't check it.

so anyone can revoke a token issued by any client.

it'd be nice to have but probably not blocking

the other two issues are repeating parameters in endpoints, like specifying the client_id or grant_type etc twice in the url.

if you have upstream fixes pending already then I think we're good for now

yup cool.

!m lucifer

You're doing good work, lucifer!

then there are recommended fixes.

Does the server require PKCE:

we don't require PKCE in MB. should we make it mandatory for MeB?

I'm not sure we can reasonably require that

i see.

(it was only quietly introduced recently)

Or at least not without an announcement.

okay, i guess we can make a ticket tracking this and update the document to strongly recommend it?

*documentation

MB support PKCE for 3 years already.

*s

ok, in 2020, but still quietly. I'm not sure how many clients use it yet.

yvanzo: the migration from MB to MeB provider will need manual intervention from app owners most likely so that announcement could cover it

so we agree to not make it mandatory for now?

what sort of manual intervention will it require?

yes, agreed

how many clients currently use MB auth?

unless it is straightforward, yes agreed.

realistically if you don't enforce PKCE now you never will :p

creating a new app on the dashboard linked before. and changing the client secrets locally.

bitmap: ^

there are 4760 rows in the application table

huh that's not a lot.

because every MB, CB, LB local installation needs at least one.

*BB, CB, LB

i suspect <5% are actively used

probably lower

only 566 have any tokens granted

and also Discourse and Jira.

i see

and Weblate.

i'd be surprised if any of those don't support PKCE

i think it could be possible to mandate pkce then. how about i create a ticket for this and collect more info to discuss later this week?

It is fine to leave a 1 year probation period if announced at the same time.

only 6 applications with non-expired tokens (?!)

the JS client in https://d.ontun.es could be an issue

sorry, i don't want to add to the pain of the migration

LB beta, LB prod, LB test, BB, CB would make 5.

(^ that is one of the 6)

i just think that this is the time do it, because you know how it goes

hopefully in 1 year there'll be loads of clients using OAuth

yea, given how few there are currently you may be right

so then there is more of an incentive not to enforce it

happy to help d.ontun.es peeps with the migration if its open source.

and i think we own all of the other clients so not an issue.

hmm interesting that BB and CB are not on that list.

It is better to have our clients using it asap in any case.

yup makes sense

But not enforcing it right now for the few clients we don’t own seem fair.

how about if we reach out to the active clients and ask them if they are fine pkce being mandatory?

number seems small enough to be feasible

worth a try

a related recommendation is to not support plain PKCE.

I’m not sure to follow what represents this list of 6.

MB OAuth clients with active access tokens afaiu

select name from application where exists (select 1 from editor_oauth_token where application = application.id and expire_time >= now());

StrawBerry Music Player already does PKCE for other providers: https://github.com/strawberrymusicplayer/strawb...

how does it come that CB is not listed?

i think its because no one has recently logged in.

I did last week

and tokens expire hourly.

right, we probably need a bigger expire window to gather an exhaustive list of clients.

Oh so that list can vary anytime.

we have a bunch of expired tokens from 2014/2015 which were never cleaned up for some reason

but yes, otherwise the list can vary a bit

we can probably check when the last token was issued to an application for a better estimate.

Does PKCE has anything to do with the app itself, or just the tokens?

you need a couple of code changes in the OAuth process to obtain the tokens with PKCE.

I mean do apps need to support it in a way?

yes code changes are needed in the clients to get the token but nothing after the fact.

There are 144 apps with token that expired since last year.

a lot of these may be one time things. like local deployments.

24 with a token since last week

would be simple to check that from the redirect_uris/name/description

yes but even then, it's not 6.

the local deployments use the code from LB, CB, BB so wouldn't need any change to support.

note that, unless we plan to shut off the MB OAuth soon (i don't think we do). mandating PKCE will only prolong the migration by some time during which we need to support both.

and that is fine i guess.

we will probably have to continue this discussion later

apologies, I didn't meant to derail the discussion

yup makes sense. after the regular meet or some other day?

but it is good to see that there won't be so many apps to move.

was there anything else to discuss?

ideally another day, I really need to work on PG upgrade stuff :)

yup loads of it :D

ah okay, makes sense.

any suggestions on when?

after that?

great work btw lucifer

one thing that stood out from the document - doesn't Picard use the OOB redirect uri?

:)

do you enjoy reading RFCs?

bitmap: yeah that is another lengthy discussion

ok, sorry for bringing it up :D

atj: certainly better than writing compliance tests for the rfcs

well, that's like me asking if you like being kicked in the balls and you telling me you prefer to be punched in the face

its a nice change sometimes but wouldn't volunteer for it for sure XD

lol

when's the schema change?

13 may

so how about 16th?

Don't call me first for reviews please, metro is slow

i am fine with any day that week or the one after that fwiw.

that tenatively sounds fine

cool

thanks reosarevok, bitmap, yvanzo and atj!

thank you!

<BANG>

And on that note, welcome to this MetaBrainz Monday Meeting!

🙋

🙋‍♀️

My list for today: Pratha-Fish, huhridge, jasje, JadedBlueEyes, pranav, Tarun_0x0, mayhem, rimskii, atj, monkey, yvanzo, yellowhatpro, theflash__, reosarevok, ansh, zas, ApeKattQuest, akshaaatt, kellnerd, lucifer, bitmap

If anyone else wants in, please let me know.

reosarevok: mind adding me too?

Sure! Today or every week?