Page MenuHome

BlenderID support in Phabricator
Confirmed, NormalPublicTO DO

Description

There are two instances of the integration:

  • Support BlenderID OAuth2 login in phabricator.
  • Support BlenderID badge system

More information needed.

Event Timeline

Dalai Felinto (dfelinto) lowered the priority of this task from 90 to Normal.Aug 29 2019, 4:11 PM
Dalai Felinto (dfelinto) created this task.

i have started to integrate the BlenderID OAuth2 for phabricator. you should be able to see something in the following days

Hi Frederic. Francesco has mentioned someone working on it. Could you give a bit more detail on that and how far you got already?

Good morning Dalai! At the moment I m testing the OAuth2 login communication between Phabricator and Blender ID. It seems to be pretty straight forward as its well implemented on both side. I m actually spending more time setting things up.
In Phabricator, i m simply extending the PhabricatorOAuth2AuthProvider class, which handle all the work.
Right now, as Blender ID require an https return URI, i have to change my local phabricator to https.
Implementing the badge system will required more coding. I have still not had a look at it, thats why the OAuth2 login is a good warm up for me :)
at the moment, I can only work a couple hours a day for Blender.

Frederic (TaOuch) added a comment.EditedSep 1 2019, 5:06 PM

HI. i have couple of questions

  • in order to have phabricator running, it needs also arcanist and libphutil. in the git.blender i see only Phabricator. i tried to used arcanist and libphutil from Phacility but then i get a missmatch version. so for now i using the one from Phacility, but what shall i do to make the one from git.blender work?
  • i get my app authorized by my local blender id. then Phabricator tries to query api/me, but my local blender id throw an error,

AttributeError: 'AnonymousUser' object has no attribute 'roles'

here is an example of the URI
http://id.local:8000/api/me?access_token=dkM1nazOTk2GR57hxCHMgy7XKho9Zb

i get same error in either curl or postman.
from the browser where i m logged it returns a nice JSON

  • the same access token works for many users. the browser returns me the data of the user which is logged. i thought an access token was tight to a user ... ?

The Phabricator repository indeed doesn't have the arcanist and libphutil as submodule. You need to clone those repositories as per Phabricator installation guide, then checkout the following sha1s:

arcanist: 7329bc7c32b995a7dce0319723857cbdf4f95a91
libphutil: ada64585035e8fbdf5d0dd6abfb6c13e6b160b7d

Installation doc: https://secure.phabricator.com/book/phabricator/article/installation_guide/ (under Installing Required Components)

thanks. it works now
in order to have a new OAuth2 provider in Phabricator, i have to add a class also in lipphutil

'PhutilBlenderIDAuthAdapter' => 'auth/PhutilBlenderIDAuthAdapter.php'

The custom code is to be places under src/extensions folder. We already have custom apps for buildbot and documentation integration.

Also, you don't need to manually add phutil library map, use arc liberate for this.

For the badge system I would imagine we would use existing Phabricator's badges, and some cronjob type of a thing to synchronize those from Blender ID using Conduit API. At least that's what seemed most clean approach when we've looked into this with @Francesco Siddi (fsiddi).

thanks. src/extensions works like a charm

Can you submit it here as a differential review (firing a patch against blender-tweaks branch of phabricator.git repository)?
There is some feedback on the code (nothing big from my side, but doing it in code review is more natural).

Francesco told me to create a standalone repo. for me its the same :) i guess check together how to handle this best

otherwise i would like to start thinking about the Badges if you ok. Sergey mentioned the possibility to do a cronjob that will take care of the sync. personally i think this is the good backup solution. but i would better check a solution within Phabricator itself. could easily hook something.

the main question is how often you want the sync to be done? only once a day? or more close to the real time?

after talking with Francesco, and as Sergey mentionned, i gonna do a python script that will query Blender ID and update Phabricator via the Conduit API. it will be triggered via a cronjob

My suggestion is to not over-engineer it. Just update the patch whenever things are ready or there is something new you need to be reviewed.

Specially because different bits of the functionality can be reviewed separately.

While the development would need to be moved to the blender-tweaks branch for the ease of deployment, here is some quick feedback about code:

  • Please strip all the trailing whitespace.
  • if (strlen($name)) { => if ($name != '') (avoid calculating length just to see if the string is empty)
  • Try to move the logo into an own file. This file *will* be changed in the upstream sooner than later, and merging our tweaks in there will not be trivial.

thanks for the feedback, i have made an update to the repo

for the badge implementation, on Phabricator side:

  • Phabricator doesnt seem to let their tool to be easily extended. Most of their classes are lock down with a 'final class'. And so is the badge system. Here are some UI examples of their badges

https://secure.phabricator.com/uiexample/view/PHUIBadgeExample/

So if you want to have for instance the Blender logo instead of their icons, some CSS tricks could be done to rewrite their CSS rules. i guess this is the easiest solution. but its ugly, and weak (as soon as Phabricator would changed their HTML tags / class, the new Blender CSS rules could be lost)

If a proper customisation is needed for the Blender Badge Layout, i guess a new Blender Badge System could be implemented in parallel to their badges system. But i am not sure if its worth it.

  • the Conduit API is a bit tricky to use and very limited, specially about the users data.

I need to retrieve the user phID from their emails, or even better, from the Blender ID user which is also saved inside Phabricator user_externalaccount table. but the last one is far beyond Conduit scope.

Conduit API let you search users from their username, and thats about it.

I could simply query directly the dB to retrieve the user phID, and then use the Conduit API to create the relation phID badge -> phID user

on the Blender ID side:

  • the Blender ID API seems to give badges information for the owner of the token only

A solution would be to retrieve all the valid token directly from the dB (Blender or Phabricator dB) for the selected users, and then use the API to get the badges for each token.
Another solution would be to update the Blender ID API with a new method that will allow to retrieve all badges. For performance, it could be wise also to be able to apply some filter like a list of Blender ID user, or the date (which is not currently save inside the Blender dB)

  • i get my app authorized by my local blender id. then Phabricator tries to query api/me, but my local blender id throw an error,

AttributeError: 'AnonymousUser' object has no attribute 'roles'
here is an example of the URI
http://id.local:8000/api/me?access_token=dkM1nazOTk2GR57hxCHMgy7XKho9Zb

You cannot pass access tokens on the URL. Use the HTTP Authorization header for this, like Authorization: Bearer dkM1nazOTk2GR57hxCHMgy7XKho9Zb.

Thanks Sybren.

The token topic has been already solved.
you can find the implementation of the Blender ID login here
https://github.com/TaOuch/phabricator-blender-id-oauth-login

From the tests with @Francesco Siddi (fsiddi).

If you go to the /settings/user/user/page/external/ page after logging in with Blender ID account the following message is shown:

OAuth2 Account·Invalid OAuth Access Token

Don't think it's something what is supposed to happen.

P.S. By accident submitted non-final text, sorry.

yes. it should have been corrected on the last push last week ... i gonna fire a patch now on the blender-tweaks branch :)

As discussed in a chat with Frederic, I have asked @Sybren A. Stüvel (sybren) to pick this up and advise Frederic if there are any further questions with the integration. Feel free to discontinue the repo at https://github.com/TaOuch/phabricator-blender-id-oauth-login and start working against blender-tweaks. I can give some more feedback about Badges later on.

to give some updates:

  • I have implemented a Phabricator lib that will update the user badges at login. I still have to clean the code before i make a diff review.
  • I still have to make the daily script that will updates all badges from all the users. Unfortunately both Blender ID API and Conduit API are not enough to be able to do the job properly.

For instance Conduit API can not search a user per email, nor per login method. and Blender ID API return only the badges associated to the owner of the used Token.

So here, i would recommend to by pass the APIs, and query directly the database. Its definitely the fastest and the cleanest solution.

I still have to make the daily script that will updates all badges from all the users.

Please make this a hourly/5-minutely thing, so that the load is spread across the day and not bundled up at one point in time.

Unfortunately both Blender ID API and Conduit API are not enough to be able to do the job properly.
For instance Conduit API can not search a user per email, nor per login method. and Blender ID API return only the badges associated to the owner of the used Token.

So what can the Conduit API do?

Instead of refreshing all badges on a daily basis, wouldn't it be easier to do it every once in a while during the user's browsing session? At that time the user is logged in, so their token is known, and it might be possible to refresh the badge in the background? Just thinking out loud here, I have no experience with Phabricator so I don't know whether this is easy/hard/impossibru.

So here, i would recommend to by pass the APIs, and query directly the database. Its definitely the fastest and the cleanest solution.

I strongly disagree here. Having Phabricator connect to the Blender ID database is not a good idea. Not only from an infrastructural standpoint (it creates tight coupling between Blender ID and Phabricator, and makes database migrations impossible) but also from a security standpoint (it contains more data than Phabricator is allowed to access right now). Furthermore, the people at the KDE project also are interested in Blender ID integration with their Phabricator installation. Giving third parties database access doesn't sound like a wise plan to me.

I am not aware of any background processes supported by Phabricator.

From quick look synchronizing badges themselves (as in, Patron, Silver, etc) should already be possible.

For attaching badges to users we can easily add a custom conduit API which will receive a Blender ID and list of badges. It will be then simple to query an actual user and modify its badges.

Frederic (TaOuch) added a comment.EditedOct 29 2019, 8:09 PM

So what can the Conduit API do?

The Conduit API can search users on id, phid, and username, but it never returns any emails.
I need to retrieve the Phabricator users who log in via Blender ID. In the Phabricator dB, table user_externalaccount, there is a field accountID which is the user ID from Blender ID. Having this data would be optimal.

https://secure.phabricator.com/conduit/method/user.search/

For the badges, Conduit API is enough. I can search a badge from his name, and award user to badge. I do not think i need more.

wouldn't it be easier to do it every once in a while during the user's browsing session?

Yes i agree. But i think @Francesco Siddi (fsiddi) was worried about performances, and pointed out having a daily cron job might be less servers intensive.

At that time the user is logged in, so their token is known, and it might be possible to refresh the badge in the background? Just thinking out loud here, I have no experience with Phabricator so I don't know whether this is easy/hard/impossibru.

Yes totally. I think its the right logic. But i havent been able to find a hook in Phabricator for doing that.

Having Phabricator connect to the Blender ID database is not a good idea

I meant that the cron job access directly the dB. It need read only users:

  • On the Phabricator side, to retrieve the users that log in via Blender ID
  • On the Blender ID side, to retrieve the badges.

To award the badges, i could then simply use the Conduit API.

Furthermore, the people at the KDE project also are interested in Blender ID integration with their Phabricator installation.

Yes, then in this case, having a cron job might not be the best solution.

(i have edited a typo mistake)

From quick look synchronizing badges themselves (as in, Patron, Silver, etc) should already be possible.

Do you have links / docs about synchronising badges?

we can easily add a custom conduit API

This is a good idea. I will check it out.

So for the rest of the week, i will further study on:

  • how to extend the Conduit API.
  • find a hook in Phabricator to be able to update the badges.

Do you have links / docs about synchronising badges?

From the phabricator there is a badge conduit API. For seeing how tow glue Phabricator with Blender ID's badges check on how devtalk does it.
Unfortunately, i am not sure where that code is.

I meant that the cron job access directly the dB.

Never do it. Schema does change, and it's out of our control.
Catching issue with changed API when used from a custom Conduit API is way easier to catch.

Hi all.

I think i have some nice progress. I have been able to hook some functions via Phabricator's events system. From there i can query the Blender ID and Conduit APIs. The problem with this solution is that Phabricator says:

The event system is an artifact of a bygone era. Use of the event system is strongly discouraged. We have been removing events since 2013 and will continue to remove events in the future.

https://secure.phabricator.com/book/phabricator/article/events/

But they do not say what are the alternatives. So anyway i m going into this direction as this seem to be the "best" solution i got at the moment. The code that will update the badges should be portable (if well coded :) ) and if later on i find a better place to hook Phabricator, i will be able to easily move it.

I have also made some progress with the understanding of Phabricator's internal structure. Retrieving users datas, access token, and external account are much more clear.

So i hope i ll be able to send a diff patch for the end of the week with the 1st version of the code to sync the Badges.

Hi

I have updated the differential
https://developer.blender.org/D6015

About the Badge updater:

  • A user Bot has to be created to reward the Badges. For now please give him the username BotBadge. Later on i can add a propriety inside the Blender ID Oauth settings to set the Bot username
  • Phabricator and Blender ID Badge have a different UI, therefore different properties. For instance in Phabricator we have the Badge quality and Badge icon which can not be picked up from Blender ID. So, the Badges have to be created manually in Phabricator.
  • The event that handle the update of the Badge ui.willRenderProperties, is triggered when the user goes to its Profile. After an update, the timestamp is saved inside the session, which will be checked to be sure no update will be done for the next 24h.

There is still couple of things to do. Like revoking Badges from users, saving the Bot username inside the Blender ID Oauth settings, but i would like a feedback before i go further down this road.

So, the Badges have to be created manually in Phabricator.

Can it be done more like a manual edit is needed? As in, system will create a badge if it's needed and then one can tweak it's icon and what not?

The event that handle the update of the Badge ui.willRenderProperties, is triggered when the user goes to its Profile.

It this how Phabricator works or is it how the patch works?

Can it be done more like a manual edit is needed? As in, system will create a badge if it's needed and then one can tweak it's icon and what not?

Yes. This can be done. I will make an update to the patch tomorrow.

It this how Phabricator works or is it how the patch works?

Phabricator triggers the event ui.willRenderProperties when the user goes to its Profile. Not that its written in the doc, but from my tests its what i found out. If you have the console enabled in Phabricator, there is a tab with the events that are triggered.
The Blender ID Badge updater register himself with this event. If this event solution satisfy you, i could try to create a specific Blender ID Badge event.

I have made an update

https://developer.blender.org/D6015

  • Bot username can now be setup inside the Auth application settings.
  • If not present, the Badges are created simply via its name. e.g. Blender Cloud or Gold Development Fund supporter.

An administrator must then adapt and tweaks the UI of the Badge itself.

I have made an update of the patch

https://developer.blender.org/D6015

It handles now the avatar from Blender ID:

  • when a user register himself in Phabricator via the Blender ID Oauth, the avatar from Blender ID will be used to set his profile picture.
  • when a user want to change his profile picture, the avatar from Blender ID will be proposed.
  • when a user check his external accounts, the Blender ID avatar will be displayed inside the Blender ID linked account.

Hi all.

I will upload this week the last updates. The library will handle Badge revocation. I m still doing the last tests to find the last bugs. But otherwise, from my side, i think this is it.

The weak points are:

  • I did not find a clean way to add icons to the Badges. All icons are defined in the final class PhabricatorBadgesIconSet. Then i guess i could rewrite some CSS rules to swap the existing icons for some more customised Blender ID icons. But this is kind of ugly.
  • The event which trigger the Badge updates could be a bit more customised. Right now it listens to the Phabricator event ui.willRenderProperties, which happens when the user goes on his profile page. A better way would be to have a Blender ID Badge event, that gets triggered when the user goes to his badges page.

I did not find a way to create such an event. The Phabricator class PhabricatorEventType can not be extended.

I m looking forward to have your feedback.
The new patch is coming in the next days.

I have made an update of the patch

https://developer.blender.org/D6015

It can also revoke Badges from users.