Verify personal data #3869

schiessle · 2017-03-16T14:07:03Z

Allow users to verify their email address, twitter account and webpage for the lookup server

mention-bot · 2017-03-16T14:07:05Z

@schiessle, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @MorrisJobke and @jancborchardt to be potential reviewers.

MorrisJobke · 2017-03-16T20:50:54Z

apps/lookup_server_connector/appinfo/app.php

@@ -28,18 +28,20 @@
 		\OC::$server->getAppDataDir('identityproof'),
 		\OC::$server->getCrypto()
 	);
+
+	$config = \OC::$server->getConfig();
+	$lookupServer = $config->getSystemValue('lookup_server', '');


No default value?

The default value is that nothing is configured and than the lookup connector will fall back to the default URL.

MorrisJobke · 2017-03-16T20:51:17Z

config/config.sample.php

+/**
+ * use a custom lookup server to publish user data
+ */
+'lookup_server' => 'https://lookup.nextcloud.com/users',


I guess the /users needs to be removed here.

MorrisJobke · 2017-03-16T20:51:50Z

apps/lookup_server_connector/lib/UpdateLookupServer.php

 	/** @var Signer */
 	private $signer;
 	/** @var IJobList */
 	private $jobList;
 	/** @var string URL point to lookup server */
-	private $lookupServer = 'https://lookup.nextcloud.com/users';
+	private $lookupServer = 'https://lookup.nextcloud.com';


Use the default while fetching from the config instead.

I prefer to have the default defined in one place instead of spread around in multiple places. This makes it much easier to change it if necessary.

The problem I see with this approach: How to set "no lookup server"?

there is always one configured lookup server. That's also the case for Nc11. You can disable the whole functionality in the admin settings, no need to play around with config.php variables.

you can still have the default server stored in a const, if you need it in several places, and use it as default parameter. It's a more intuitive and especially consistent.

nickvergessen · 2017-03-17T12:12:28Z

apps/lookup_server_connector/lib/UpdateLookupServer.php

 								Signer $signer,
-								IJobList $jobList) {
+								IJobList $jobList,
+								$lookupServer = '') {


Related to: #3869 (comment)
In most cases people will just use the default value. People who use this class shouldn't have to think about it or even know the default server.

schiessle · 2017-03-20T13:15:34Z

Thanks for the early review. But keep in mind that most code will probably change before it is ready to review. That's a early development branch.

schiessle · 2017-04-06T14:11:36Z

@jancborchardt please don't forget the icons we discussed during the hackweek.... Thanks!

MorrisJobke · 2017-04-07T17:51:42Z

@jancborchardt Please fix the layout:

schiessle · 2017-04-10T13:00:04Z

@MorrisJobke for now the text label are just there to allow me to work on the actual functionality, I didn't put a lot of energy in getting the layout right (although I have to say that it looks better on my screen 😉 ).

Still waiting for some nice icons by @jancborchardt. Bonus points for Jan if he adds them directly to this branch with the right layout 😉

codecov · 2017-04-18T11:00:55Z

Codecov Report

Merging #3869 into master will decrease coverage by 23.05%.
The diff coverage is 0%.

@@              Coverage Diff              @@
##             master    #3869       +/-   ##
=============================================
- Coverage     54.34%   31.29%   -23.06%     
- Complexity    21998    22058       +60     
=============================================
  Files          1359     1360        +1     
  Lines         84340    84009      -331     
  Branches       1335     1335               
=============================================
- Hits          45838    26290    -19548     
- Misses        38502    57719    +19217

Impacted Files	Coverage Δ	Complexity Δ
settings/personal.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
settings/templates/personal.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
lib/private/Accounts/Hooks.php	`0% <0%> (-100%)`	`14 <0> (ø)`
settings/Controller/UsersController.php	`0% <0%> (-64.03%)`	`94 <7> (+7)`
lib/private/Accounts/AccountManager.php	`0% <0%> (-68.19%)`	`34 <25> (+24)`
settings/BackgroundJobs/VerifyUserData.php	`0% <0%> (ø)`	`25 <25> (?)`
...es_sharing/lib/Controller/ShareesAPIController.php	`0% <0%> (-68.44%)`	`101 <0> (ø)`
...lookup_server_connector/lib/UpdateLookupServer.php	`0% <0%> (ø)`	`19 <0> (+5)`	⬆️
apps/lookup_server_connector/appinfo/app.php	`0% <0%> (ø)`	`0 <0> (ø)`	⬇️
...p_server_connector/lib/BackgroundJobs/RetryJob.php	`0% <0%> (ø)`	`6 <3> (ø)`	⬇️
... and 368 more

MorrisJobke · 2017-04-18T21:12:18Z

@jancborchardt Please have a look at this or this will not land in 12.

jancborchardt · 2017-04-23T02:44:59Z

Sorry, have been drowning in notifications – will check it out.

schiessle · 2017-04-24T09:25:35Z

@jancborchardt Thanks. As discussed we need three icons:

not verified
verification in progress
verified

schiessle · 2017-04-24T10:25:48Z

@jancborchardt I just pushed the last changes for the code. The strings "verification in progress", "verified", "click to verify" here 24e1395#diff-e999cb367e8eac4edfd688b687c55730L114 need to be replaced with nice looking icons. Maybe it make sense to keep the strings as a tooltip.

Email has the status "verification in progress" and "verifed" only and is not clickable because email verification doesn't need any additional user interaction. The lookup server send automatically a verification mail if you set an new email address and we check in the background if the verification was completed.

jancborchardt · 2017-04-25T13:40:25Z

@schiessle what do you think about these icons:

Click to verify
Verification in progress
Verified
and 5: Error state – which one do you prefer?

schiessle · 2017-04-25T15:33:14Z

@jancborchardt looks nice. I'm not sure if we need a error state. My plan was to just go back from "verify in progress" to "not verified" and then the user can start again. Can you commit the icons? Thanks!

jancborchardt · 2017-04-25T16:05:07Z

@schiessle I added the icons and also would say we simplify the wording to:

Verify (cause »click« isn’t on mobile ;)
Verifying …
Verified

Can you make all the changes for that, let me know, and then I fix the last layout issues?

schiessle · 2017-04-26T08:04:57Z

@jancborchardt verification buttons are in and they look really nice! 😃

Can you have a look at the layout, there are still some glitches on different window sizes. Thanks!

Signed-off-by: Bjoern Schiessle <[email protected]>

…actual value Signed-off-by: Bjoern Schiessle <[email protected]>

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

…n some value Signed-off-by: Bjoern Schiessle <[email protected]>

Signed-off-by: Bjoern Schiessle <[email protected]>

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Signed-off-by: Bjoern Schiessle <[email protected]>

… not only after page reload Signed-off-by: Bjoern Schiessle <[email protected]>

Signed-off-by: Bjoern Schiessle <[email protected]>

…eady running Signed-off-by: Bjoern Schiessle <[email protected]>

Signed-off-by: Bjoern Schiessle <[email protected]>

Signed-off-by: Roeland Jago Douma <[email protected]>

Signed-off-by: Bjoern Schiessle <[email protected]>

MorrisJobke · 2017-04-29T02:42:08Z

I rebased to trigger CI - restarting is not possible for the jobs before the drone mess up. :/

MorrisJobke

Tested and works:+1:

Signed-off-by: Morris Jobke <[email protected]>

MorrisJobke · 2017-04-29T03:38:00Z

unit tests run fine -> merge

schiessle added 2. developing Work in progress feature: federation labels Mar 16, 2017

schiessle added this to the Nextcloud 12.0 milestone Mar 16, 2017

MorrisJobke reviewed Mar 16, 2017

View reviewed changes

MariusBluem mentioned this pull request Mar 16, 2017

Select own Lookup Server #3127

Closed

nickvergessen reviewed Mar 17, 2017

View reviewed changes

schiessle force-pushed the verify-personal-data branch from f7cc86c to 257a8d9 Compare March 20, 2017 15:29

nickvergessen mentioned this pull request Mar 27, 2017

Nextcloud 12 overview #2672

Closed

59 tasks

schiessle force-pushed the verify-personal-data branch from 257a8d9 to 046d2fe Compare April 6, 2017 14:11

schiessle force-pushed the verify-personal-data branch from 046d2fe to f958c42 Compare April 6, 2017 15:51

MorrisJobke assigned jancborchardt Apr 7, 2017

schiessle force-pushed the verify-personal-data branch from f958c42 to 969d316 Compare April 18, 2017 11:00

schiessle force-pushed the verify-personal-data branch 2 times, most recently from 48c6ae2 to 69d179f Compare April 21, 2017 16:01

schiessle and others added 21 commits April 28, 2017 23:41

fix email verification status

5fa0e6d

Signed-off-by: Bjoern Schiessle <[email protected]>

show and hide verification dialog

a0ca1c0

Signed-off-by: Bjoern Schiessle <[email protected]>

only show verification buttons for public fields and field with some …

47985a1

…actual value Signed-off-by: Bjoern Schiessle <[email protected]>

Improve placeholders for website and Twitter handle

de529b9

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Fix dialog width

e40bdcd

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

Fix verification button placement on mobile

9c0414b

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

only show verify button if scope is public and the input field contai…

ec452a8

…n some value Signed-off-by: Bjoern Schiessle <[email protected]>

update message in verify popup

e9021c1

Signed-off-by: Bjoern Schiessle <[email protected]>

Fix text overflow in website popover

4607916

Signed-off-by: Jan-Christoph Borchardt <[email protected]>

only close verification dialog if the user clicks outside of the dialog

2d07179

Signed-off-by: Bjoern Schiessle <[email protected]>

fix name of website input field

072d69f

Signed-off-by: Bjoern Schiessle <[email protected]>

verification button should be clickable directly after status change,…

68ba857

… not only after page reload Signed-off-by: Bjoern Schiessle <[email protected]>

move translatable strings out of the template

9480b29

Signed-off-by: Bjoern Schiessle <[email protected]>

allow to display the verification code also while verification is alr…

806ca43

…eady running Signed-off-by: Bjoern Schiessle <[email protected]>

use already available instance if l10n

df1d1c6

Signed-off-by: Bjoern Schiessle <[email protected]>

cleanup template

4ec05ef

Signed-off-by: Bjoern Schiessle <[email protected]>

don't add multiple retry jobs

7340d6a

Signed-off-by: Bjoern Schiessle <[email protected]>

fix path to the website proof

5464b14

Signed-off-by: Bjoern Schiessle <[email protected]>

fix unit test

9b36f2d

Signed-off-by: Bjoern Schiessle <[email protected]>

Updated autoloader

e3b10f3

Signed-off-by: Roeland Jago Douma <[email protected]>

send signature to lookup server

f488258

Signed-off-by: Bjoern Schiessle <[email protected]>

MorrisJobke force-pushed the verify-personal-data branch from 390be0c to f488258 Compare April 29, 2017 02:41

MorrisJobke approved these changes Apr 29, 2017

View reviewed changes

Fix unit tests

a0bf706

Signed-off-by: Morris Jobke <[email protected]>

MorrisJobke merged commit 1307800 into master Apr 29, 2017

jancborchardt deleted the verify-personal-data branch April 29, 2017 13:39

virtualdxs mentioned this pull request Nov 12, 2018

.well-known/CloudIdVerificationCode.txt not registered with IANA #11623

Closed

nunesgh mentioned this pull request Jan 18, 2021

.well-known/CloudIdVerificationCode.txt feature seems to be broken #25196

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Verify personal data #3869

Verify personal data #3869

schiessle commented Mar 16, 2017 •

edited by nickvergessen

Loading

mention-bot commented Mar 16, 2017

MorrisJobke Mar 16, 2017

schiessle Mar 20, 2017

MorrisJobke Mar 16, 2017

MorrisJobke Mar 16, 2017

schiessle Mar 20, 2017 •

edited

Loading

MorrisJobke Apr 6, 2017

schiessle Apr 7, 2017

blizzz Apr 26, 2017

nickvergessen Mar 17, 2017

schiessle Mar 20, 2017

schiessle commented Mar 20, 2017

schiessle commented Apr 6, 2017

MorrisJobke commented Apr 7, 2017

schiessle commented Apr 10, 2017

codecov bot commented Apr 18, 2017 •

edited

Loading

MorrisJobke commented Apr 18, 2017

jancborchardt commented Apr 23, 2017

schiessle commented Apr 24, 2017

schiessle commented Apr 24, 2017

jancborchardt commented Apr 25, 2017

schiessle commented Apr 25, 2017

jancborchardt commented Apr 25, 2017

schiessle commented Apr 26, 2017

MorrisJobke commented Apr 29, 2017

MorrisJobke left a comment

MorrisJobke commented Apr 29, 2017

Verify personal data #3869

Verify personal data #3869

Conversation

schiessle commented Mar 16, 2017 • edited by nickvergessen Loading

mention-bot commented Mar 16, 2017

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

schiessle Mar 20, 2017 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schiessle commented Mar 20, 2017

schiessle commented Apr 6, 2017

MorrisJobke commented Apr 7, 2017

schiessle commented Apr 10, 2017

codecov bot commented Apr 18, 2017 • edited Loading

Codecov Report

MorrisJobke commented Apr 18, 2017

jancborchardt commented Apr 23, 2017

schiessle commented Apr 24, 2017

schiessle commented Apr 24, 2017

jancborchardt commented Apr 25, 2017

schiessle commented Apr 25, 2017

jancborchardt commented Apr 25, 2017

schiessle commented Apr 26, 2017

MorrisJobke commented Apr 29, 2017

MorrisJobke left a comment

Choose a reason for hiding this comment

MorrisJobke commented Apr 29, 2017

schiessle commented Mar 16, 2017 •

edited by nickvergessen

Loading

schiessle Mar 20, 2017 •

edited

Loading

codecov bot commented Apr 18, 2017 •

edited

Loading