anaynayak / buildnotify

A system tray based build status notification app for cctray.xml feeds.
https://anaynayak.github.io/buildnotify/
Other
29 stars 7 forks source link

Add support for Bearer token #121

Closed Deuchnord closed 4 years ago

Deuchnord commented 5 years ago

I'm using Drone CI for my personal development, which uses a Bearer token associated to the user account to authenticate and access the CCMenu entrypoint:

curl --request GET \
  --url https://example.com/api/badges/Me/Project/cc.xml \
  --header 'authorization: Bearer aBcDeFgHiJkLmNoPqRsTuVwXyZ0123456789'

From a point of view of security, using a Bearer token is not a bad idea. Is it possible to add support for Bearer tokens instead of the classic user/password?

anaynayak commented 5 years ago

@Deuchnord thanks for reporting this. Yes it should be possible to add this support. Feel free to raise a PR if you want to. I'm a bit busy in the near term but should be able to get it in a week or so.

Deuchnord commented 5 years ago

I'm not an expert of Python and Qt but I'll try to do this :)

luchiago commented 5 years ago

Hi, this issue still need help?

Deuchnord commented 5 years ago

I'm still working on it but didn't take the time to create a PR. I made another one before to enhance the windows (#124), to make this issue easier to develop (especially placing the new UI elements). Would be nice if it could be reviewed :slightly_smiling_face:

anaynayak commented 5 years ago

Sorry. I'll be able to get to that PR tonight or early morning. Currently don't have access to a laptop.

On Tue, Oct 8, 2019, 1:19 PM Jérôme Deuchnord notifications@github.com wrote:

I'm still working on it but didn't take the time to create a PR. I made another one before to enhance the windows (#124 https://github.com/anaynayak/buildnotify/pull/124), to make this issue easier to develop (especially placing the new UI elements). Would be nice if it could be reviewed 🙂

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/anaynayak/buildnotify/issues/121?email_source=notifications&email_token=AADFTU6CRS7IYKTASDZQ5CDQNQ3P3A5CNFSM4I2ECTJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEATH2II#issuecomment-539393313, or mute the thread https://github.com/notifications/unsubscribe-auth/AADFTUZF2FDLMFXBPGUVSE3QNQ3P3ANCNFSM4I2ECTJQ .

anaynayak commented 5 years ago

I'm a bit surprised with the use of Bearer auth by Drone CI. Don't think the others CI providers use this and it has mostly been just basic auth or query param based.

@Deuchnord for the ui elements, Are you thinking of this being an additional label/text combination ? I almost feel the need to hide this somewhere(collapsible/advanced settings) so that everyone else doesn't see few extra options.

Deuchnord commented 5 years ago

@Deuchnord for the ui elements, Are you thinking of this being an additional label/text combination ? I almost feel the need to hide this somewhere(collapsible/advanced settings) so that everyone else doesn't see few extra options.

I was actually thinking about a combo box in the credential part, with two options:

If the token option is chosen, then the Username field disappears, and the Password field is renamed "Token" (it's only aesthetics, the logic remains the same). Something like this:

Username/password fields

Bearer token field

Just one question remains: should we hide the token like we do for passwords, or can it be displayed normally?

anaynayak commented 5 years ago

Thanks for the screenshots. Very clear 👍

We should definitely hide the token like for passwords since this I believe this can be used for other actions on behalf of the user. same for storage via keychain if available.