Add Keybase integration #10297

Gargron · 2019-03-16T23:59:40Z

Allow users to setup a cryptographic connection between their Mastodon profile and their Keybase profile. Keybase is extracted into a polymorphous ProofProvider interface to potentially allow other providers to be supported and minimize "hardcoding" a specific provider.

Performance notes:

One synchronous HTTP request is performed when proof is being saved to check if the signature hash is actually valid on Keybase
One asynchronous HTTP request is queued right after the proof is created to check if the proof went live on Keybase (and can therefore be showcased)
One synchronous HTTP request is performed to retrieve the avatar URL from Keybase for each proof, both on the proof index as well as on new proof page
One synchronous HTTP request is performed to check if the proof is still live and valid whenever the proof index is loaded

Resolve #10013

…ng with proofs.

URLs without params had a trailing `?`.

…aying

app/lib/proof_provider/keybase.rb

app/lib/proof_provider/keybase/config_serializer.rb

spec/controllers/api/proofs_controller_spec.rb

app/lib/proof_provider/keybase/config_serializer.rb

Gargron · 2019-03-17T19:18:53Z

app/controllers/settings/identity_proofs_controller.rb

+
+  def index
+    @proofs = AccountIdentityProof.where(account: current_account).order(provider: :asc, provider_username: :asc)
+    @proofs.each(&:refresh!)


@xgess Is this necessary? n synchronous HTTP requests on page load is not great. I would think the worker queued after the proof is saved would take care of checking if the proof is live

The refresh! do not seem to be synchronous (it spawns a worker?), but I question the need to trigger a refresh each time that page is visited.

It does worker_class.new.perform(@proof) which is synchronous

the main reason i had to do this was that there's nothing currently built on the mastodon side to recognize remotely revoked proofs. i thought it might be weird if a user revokes a proof in keybase, then days later still sees it as live in mastodon until the first refresh. we had it as a note to talk about / build something that might let keybase inform mastodon a proof is revoked, or build a rake task for mastodon to check and invalidate revoked proofs.

i'm flexible on this though. if you want to change it to be async, i think that's reasonable.

ClearlyClaire

This review doesn't really cover the correctness of the implementation wrt. keybase's protocol itself, as I have not read it, but only code quality and some concerns.

It seems fine overall, but I'm a bit concerned with the various HTTP calls, some of which synchronous. This shouldn't be a major issue, but still.

Also, will this work for any instance, or do instance admins still need to get approved by keybase?

Wouldn't it make sense to have a setting to enable or disable this feature? (Especially if the admin needs to take action wrt. keybase for it to be usable)

config/settings.yml

ClearlyClaire · 2019-03-18T17:35:38Z

app/controllers/settings/identity_proofs_controller.rb

+    @proof = current_account.identity_proofs.new(
+      token: params[:token],
+      provider: params[:provider],
+      provider_username: params[:provider_username]


resource_params?

Ah no, the difference to resource_params is that these are like ?token=??? while resource_params after form submission are nested

ClearlyClaire · 2019-03-18T17:39:27Z

app/lib/proof_provider/keybase.rb

+
+    if verifier.valid?
+      @proof.verified = true
+      @proof.live     = false


Should ActiveRecord validation really have side-effects?

I suppose you could argue that verified could be set before_save which would not happen if validation doesn't pass, but I am not too unhappy with how it is

app/views/accounts/_bio.html.haml

ClearlyClaire · 2019-03-18T17:50:19Z

app/controllers/settings/identity_proofs_controller.rb

+
+  def index
+    @proofs = AccountIdentityProof.where(account: current_account).order(provider: :asc, provider_username: :asc)
+    @proofs.each(&:refresh!)


The refresh! do not seem to be synchronous (it spawns a worker?), but I question the need to trigger a refresh each time that page is visited.

Gargron · 2019-03-18T18:05:25Z

Also, will this work for any instance, or do instance admins still need to get approved by keybase?

Wouldn't it make sense to have a setting to enable or disable this feature? (Especially if the admin needs to take action wrt. keybase for it to be usable)

I was assured no proactive action would be necessary on the part of the instance admin

witcheslive · 2019-03-19T01:20:29Z

I have updated witches.live to the master branch which has support for this, do you need any help testing it, and if so how could I go about doing that?

cnf · 2019-03-23T22:01:05Z

Is there any documentation on how to use this as a user? I can't seem to find it.

Gargron · 2019-03-24T03:49:08Z

@cnf It is not live on Keybase's side yet. Other than that, there is nothing to do on Mastodon, you initiate the process from Keybase.

cnf · 2019-03-24T12:09:51Z

Right, thanks!

* create account_identity_proofs table * add endpoint for keybase to check local proofs * add async task to update validity and liveness of proofs from keybase * first pass keybase proof CRUD * second pass keybase proof creation * clean up proof list and add badges * add avatar url to keybase api * Always highlight the “Identity Proofs” navigation item when interacting with proofs. * Update translations. * Add profile URL. * Reorder proofs. * Add proofs to bio. * Update settings/identity_proofs front-end. * Use `link_to`. * Only encode query params if they exist. URLs without params had a trailing `?`. * Only show live proofs. * change valid to active in proof list and update liveness before displaying * minor fixes * add keybase config at well-known path * extremely naive feature flagging off the identity proof UI * fixes for rubocop * make identity proofs page resilient to potential keybase issues * normalize i18n * tweaks for brakeman * remove two unused translations * cleanup and add more localizations * make keybase_contacts an admin setting * fix ExternalProofService my_domain * use Addressable::URI in identity proofs * use active model serializer for keybase proof config * more cleanup of keybase proof config * rename proof is_valid and is_live to proof_valid and proof_live * cleanup * assorted tweaks for more robust communication with keybase * Clean up * Small fixes * Display verified identity identically to verified links * Clean up unused CSS * Add caching for Keybase avatar URLs * Remove keybase_contacts setting

xgess and others added 30 commits March 5, 2019 17:44

create account_identity_proofs table

b664211

add endpoint for keybase to check local proofs

73b9867

add async task to update validity and liveness of proofs from keybase

be92df5

first pass keybase proof CRUD

391a714

second pass keybase proof creation

5554b60

clean up proof list and add badges

fd670b5

add avatar url to keybase api

09886f5

Always highlight the “Identity Proofs” navigation item when interacti…

90aa86b

…ng with proofs.

Update translations.

bed4005

Add profile URL.

b7af7d9

Reorder proofs.

82e7a46

Add proofs to bio.

cc09a00

Update settings/identity_proofs front-end.

3c42d2d

Use link_to.

9451e8f

Only encode query params if they exist.

3c7a42f

URLs without params had a trailing `?`.

Only show live proofs.

4b341b1

change valid to active in proof list and update liveness before displ…

5781466

…aying

minor fixes

c8dc078

add keybase config at well-known path

729f6ac

fixes for rubocop

6ce8f39

extremely naive feature flagging off the identity proof UI

29c390b

make identity proofs page resilient to potential keybase issues

9476b28

normalize i18n

0bf7c56

tweaks for brakeman

2a41def

remove two unused translations

cda6394

cleanup and add more localizations

dc2ed71

make keybase_contacts an admin setting

cf214d3

fix ExternalProofService my_domain

a93123f

use Addressable::URI in identity proofs

0b3a105

use active model serializer for keybase proof config

dcf94d6

krainboltgreene reviewed Mar 17, 2019

View reviewed changes

app/lib/proof_provider/keybase.rb Show resolved Hide resolved

krainboltgreene reviewed Mar 17, 2019

View reviewed changes

app/lib/proof_provider/keybase.rb Show resolved Hide resolved

krainboltgreene reviewed Mar 17, 2019

View reviewed changes

app/lib/proof_provider/keybase/config_serializer.rb Outdated Show resolved Hide resolved

krainboltgreene reviewed Mar 17, 2019

View reviewed changes

spec/controllers/api/proofs_controller_spec.rb Outdated Show resolved Hide resolved

BenLubar reviewed Mar 17, 2019

View reviewed changes

app/lib/proof_provider/keybase/config_serializer.rb Show resolved Hide resolved

Gargron added 3 commits March 17, 2019 19:49

Merge branch 'master' into feature-keybase

de387b2

Small fixes

b343e6a

Display verified identity identically to verified links

2326745

Gargron commented Mar 17, 2019

View reviewed changes

app/lib/proof_provider/keybase/config_serializer.rb Outdated Show resolved Hide resolved

Clean up unused CSS

bbd36e0

Gargron commented Mar 17, 2019

View reviewed changes

Add caching for Keybase avatar URLs

baf695a

Gargron force-pushed the feature-keybase branch from 9477644 to baf695a Compare March 17, 2019 19:39

ClearlyClaire reviewed Mar 18, 2019

View reviewed changes

krainboltgreene approved these changes Mar 18, 2019

View reviewed changes

Remove keybase_contacts setting

ec56376

Gargron merged commit 9c4cbdb into master Mar 18, 2019

Gargron deleted the feature-keybase branch March 18, 2019 20:00

xgess mentioned this pull request Mar 19, 2019

Add Keybase Proofs xgess/mastodon#1

Closed

lapo-luchini mentioned this pull request Apr 17, 2019

[RFE] Keybase integration friendica/friendica#7016

Open

neet mentioned this pull request Jun 6, 2019

Support Identity proofs neet/masto.js#55

Merged

patcon mentioned this pull request Sep 16, 2019

Add support for keybase identity proofs loomio/loomio#5884

Open

KisaragiEffective mentioned this pull request Jun 17, 2024

Keybase対応 misskey-dev/misskey#4694

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Keybase integration #10297

Add Keybase integration #10297

Gargron commented Mar 16, 2019 •

edited

Loading

Gargron Mar 17, 2019

ClearlyClaire Mar 18, 2019

Gargron Mar 18, 2019

xgess Mar 18, 2019

ClearlyClaire left a comment

ClearlyClaire Mar 18, 2019

Gargron Mar 18, 2019

ClearlyClaire Mar 18, 2019

Gargron Mar 18, 2019

ClearlyClaire Mar 18, 2019

Gargron commented Mar 18, 2019

witcheslive commented Mar 19, 2019

cnf commented Mar 23, 2019

Gargron commented Mar 24, 2019

cnf commented Mar 24, 2019

Add Keybase integration #10297

Add Keybase integration #10297

Conversation

Gargron commented Mar 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ClearlyClaire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gargron commented Mar 18, 2019

witcheslive commented Mar 19, 2019

cnf commented Mar 23, 2019

Gargron commented Mar 24, 2019

cnf commented Mar 24, 2019

Gargron commented Mar 16, 2019 •

edited

Loading