oSumAtrIX commented 5 months ago

About

This PR builds on https://github.com/ReVanced/revanced-api/issues/150 to display GPG keys for members:

Todo

[x] Merge https://github.com/ReVanced/revanced-api/issues/150 to main
[ ] Remove hardcoded keys for testing
[ ] Fix DOM semantics
[x] Properly centre tooltip
[x] Use correct M3 guidelines for tooltip
[ ] Add a way for mobile users to view the tooltip
[ ] Fix overflow on narrow screen space

revanced-bot commented 5 months ago

Deployed at https://5911eb71.revanced.pages.dev.

PalmDevs commented 5 months ago

Design wise, it looks pretty okay. Although, I think it'd be better to put the keys into a <ul> or <ol> and align all text to the left.

Usability isn't that good right now but you've already noticed the issue, so I won't repeat the same thing.

oSumAtrIX commented 5 months ago

GPG keys can be very long, this has to be dealt with as well, usually they are displayed in armored representation of 16 hex chars length, but I am not sure how do enforce that on the website or API side as the GitHub, which is used as the source for the keys returns them in plain text

oSumAtrIX commented 5 months ago

Moving the badge to the contributors tab leaves no proper place for the badge to be shown and it gets lost between all the other contributors or you'd have to display it for all of them which deviates from what we want and loses the the purpose of adding the badge.

The donation page was chosen as it is currently the only place the website hits the members endpoint which returns information about the team. It is done on the donation page to see who you are donating too. Look up GitHub Sponsors for a native implementation of that done by GitHub.

To see the motivation to why GPG keys are added to members, check on this: https://github.com/ReVanced/revanced-api/issues/150#issuecomment-1881897951

oSumAtrIX commented 5 months ago

I am not sure what the exact difference is between the key visible in your suggested URL and the one the API will return. Users can have a list of keys which is why I don't know how that would work on your URL. Apart from that, it seems like your URL returns the full public key whereas the API will return some short form of it. If there is a reason to use your URL over what is returned by the API. Please give it a comment with a reason on the issue I sent in my previous comment. In the case we return that, we either may want to separate the endpoint as it is gonna take up most of the requests size but usually not be necessary. Additionally, the website would have to rethink how to display it die to its length

KTibow commented 5 months ago

Okay, well that lets me update my questions.

a. why are we showing that a user is verified? probably to give the feeling of security b. why are gpg keys shown? probably because they're used to sign commits c. why is the team shown? to show who you're donating to

But I'm still not sure why I would want a GPG key when GitHub already tells me if a commit is signed ("verified") using my uploaded GPG key. I guess if this is just security theater it would work fine showing the short form of the key though.

oSumAtrIX commented 5 months ago

a. As referenced in one my previous comment, it is a common security practice. If you trust the domain revanced.app, you can transitively trust keys published there. If someone signs something with one of these keys, you can transitively trust them. An example scenario would be that you can prove in an arbitrary place on the web that you are who you claim to be on revanced.app. Inheritently, the trust chain starts at revanced.app and ends with your sign, which means whoever revanced.app claims this key belongs to is not verifiable without any additional means. Because it is not important who is behind the key, and instead it is only important that you are someone trusted on revanced.app, the keys are published as a verification method. b. The GPG keys are shown to prove your relationship with revanced.app c. ✔️

But I'm still not sure why I would want a GPG key when GitHub already tells me if a commit is signed ("verified") using my uploaded GPG key.

This question is answered here:

As GitHub is a third party platform it is not where we would want to publish the keys primarily, unlike our first party platform @ revanced.app. People would not need to visit GitHub to verify that you on Discord are who you claim to be on ReVanced, dependently of GitHub whereas the ultimate fallback and source of truth, where the user consumes from, is the revanced.app domain. It goes as far as that the user also is not dependant on GitHub in terms of the source code we write, as all source is available on git.revanced.app as well. GPG signing in itself has nothing to do with code, as to why it is not a good idea to only make it accessible in relation to that. Instead it is good to present them in a primary accessible location such as the website, as the key can be used to verify your identity for any general purpose.

KTibow commented 5 months ago

Okay, fair enough. Maybe in some later day contributors/team/donate would have a different organization but I'm fine with putting them there.

Now on the topic of displaying them, I think it could work better to have a dialog triggered either by a separate button or clicking on the badge instead. What's your opinion on that?

oSumAtrIX commented 5 months ago

Okay, fair enough. Maybe in some later day contributors/team/donate would have a different organization but I'm fine with putting them there.

If you want please open an issue. Open to suggestions :)

Now on the topic of displaying them, I think it could work better to have a dialog triggered either by a separate button or clicking on the badge instead. What's your opinion on that?

I think it is expected to hover on the badge to open the tooltip. If a popup were to be shown, a click should be used. On mobile, the badge can be expanded to look like this (with the icon for example):

KTibow commented 5 months ago

Okay, sounds like a plan. I have to go but ~~when I get back~~ next morning I guess I can take a look at the code. (One more thing, given I'm assigned are you expecting me to a. leave a review as is b. wait until it's finished and leave a review then c. update the code to finish it?)

oSumAtrIX commented 5 months ago

It would be good if you could look into resolving the todos and push it. I have had my attempts but didn't really succeed in them

revanced-bot commented 5 months ago

Deployed at https://29e2d9a0.revanced.pages.dev.

revanced-bot commented 5 months ago

Deployed at https://89729c68.revanced.pages.dev.

revanced-bot commented 5 months ago

Deployed at https://0dc44863.revanced.pages.dev.

revanced-bot commented 5 months ago

Deployed at https://head.revanced.pages.dev.