Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Verify personal data #3869

Merged
merged 50 commits into from
Apr 29, 2017
+957 −56
Merged

Verify personal data #3869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
1fc05ea
allow admin to configure lookup server
schiessle Feb 22, 2017
c9ccdca
add verification dialog
schiessle Mar 14, 2017
6f41a3e
store verification status in accounts table and update lookup server
schiessle Mar 23, 2017
cbf5acc
check verification proof and update account table
schiessle Apr 21, 2017
866e5d6
use configured lookup server
schiessle Apr 21, 2017
94df091
send local verification status to lookup server
schiessle Apr 21, 2017
86701dc
make sure that we don't lose previous verification status
schiessle Apr 21, 2017
7c309c2
ask lookup server if email address was verified by the user
schiessle Apr 21, 2017
f3c433a
check right location to verify web page and query lookup server for e…
schiessle Apr 21, 2017
391a989
make sure that every data set has a 'verified' status
schiessle Apr 24, 2017
5c20288
pass verified status to template
schiessle Apr 24, 2017
f32304f
remove unused variable
schiessle Apr 24, 2017
71657db
updated unit tests
schiessle Apr 24, 2017
2a07ec8
Add verification icons
jancborchardt Apr 25, 2017
06265fe
make use of verification buttons
schiessle Apr 26, 2017
ebcb847
remove action class once the status changes from 'to verify' to 'veri…
schiessle Apr 26, 2017
46dd549
change status indicator if value was changed
schiessle Apr 26, 2017
17ad812
fix css file
schiessle Apr 26, 2017
08a33cc
Fix styling of verification box to popover
jancborchardt Apr 26, 2017
9a366db
add missing translation and other small fixes
schiessle Apr 26, 2017
67057f7
minor style fixes; use verify-action class only for verification-stat…
schiessle Apr 26, 2017
6a48548
Group verification CSS
jancborchardt Apr 26, 2017
59e8a19
Use individual verification popover dialogs instead of one
jancborchardt Apr 26, 2017
27b676b
only verify-actions should show a mouse pointer
schiessle Apr 26, 2017
ee61570
Fix email verification cursor thing properly
jancborchardt Apr 26, 2017
75c5bec
email doesn't need a verification dialog, verification mail is send a…
schiessle Apr 26, 2017
c1a784e
Fix verification icon positon and cursor
jancborchardt Apr 26, 2017
981c110
Use JS for verification icon tooltip
jancborchardt Apr 26, 2017
5fa0e6d
fix email verification status
schiessle Apr 26, 2017
a0ca1c0
show and hide verification dialog
schiessle Apr 26, 2017
47985a1
only show verification buttons for public fields and field with some …
schiessle Apr 26, 2017
de529b9
Improve placeholders for website and Twitter handle
jancborchardt Apr 26, 2017
e40bdcd
Fix dialog width
jancborchardt Apr 26, 2017
9c0414b
Fix verification button placement on mobile
jancborchardt Apr 26, 2017
ec452a8
only show verify button if scope is public and the input field contai…
schiessle Apr 26, 2017
e9021c1
update message in verify popup
schiessle Apr 26, 2017
4607916
Fix text overflow in website popover
jancborchardt Apr 26, 2017
2d07179
only close verification dialog if the user clicks outside of the dialog
schiessle Apr 26, 2017
072d69f
fix name of website input field
schiessle Apr 26, 2017
68ba857
verification button should be clickable directly after status change,…
schiessle Apr 27, 2017
9480b29
move translatable strings out of the template
schiessle Apr 27, 2017
806ca43
allow to display the verification code also while verification is alr…
schiessle Apr 27, 2017
df1d1c6
use already available instance if l10n
schiessle Apr 27, 2017
4ec05ef
cleanup template
schiessle Apr 27, 2017
7340d6a
don't add multiple retry jobs
schiessle Apr 27, 2017
5464b14
fix path to the website proof
schiessle Apr 27, 2017
9b36f2d
fix unit test
schiessle Apr 28, 2017
e3b10f3
Updated autoloader
rullzer Apr 28, 2017
f488258
send signature to lookup server
schiessle Apr 28, 2017
a0bf706
Fix unit tests
MorrisJobke Apr 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/federatedfilesharing/js/settings-personal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ $(document).ready(function() {
}
});

/* Verification icon tooltip */
$('#personal-settings-container .verify img').tooltip({placement: 'bottom', trigger: 'hover'});

$('#fileSharingSettings .clipboardButton').tooltip({placement: 'bottom', title: t('core', 'Copy'), trigger: 'hover'});

// Clipboard!
Expand Down
4 changes: 3 additions & 1 deletion apps/files_sharing/lib/Controller/ShareesAPIController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -654,13 +654,15 @@ protected function getEmail($search) {

protected function getLookup($search) {
$isEnabled = $this->config->getAppValue('files_sharing', 'lookupServerEnabled', 'no');
$lookupServerUrl = $this->config->getSystemValue('lookup_server', 'https://lookup.nextcloud.com');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rtrim('/') because people will mostlikely add a trailing slash, which might or might not cause problems...

$lookupServerUrl = rtrim($lookupServerUrl, '/');
$result = [];

if($isEnabled === 'yes') {
try {
$client = $this->clientService->newClient();
$response = $client->get(
'https://lookup.nextcloud.com/users?search=' . urlencode($search),
$lookupServerUrl . '/users?search=' . urlencode($search),
[
'timeout' => 10,
'connect_timeout' => 3,
Expand Down
16 changes: 11 additions & 5 deletions apps/lookup_server_connector/appinfo/app.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,24 @@
\OC::$server->getAppDataDir('identityproof'),
\OC::$server->getCrypto()
);

$config = \OC::$server->getConfig();
$lookupServer = $config->getSystemValue('lookup_server', '');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No default value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


$updateLookupServer = new \OCA\LookupServerConnector\UpdateLookupServer(
new \OC\Accounts\AccountManager(\OC::$server->getDatabaseConnection(), \OC::$server->getEventDispatcher()),
\OC::$server->getConfig(),
\OC::$server->getSecureRandom(),
new \OC\Accounts\AccountManager(
\OC::$server->getDatabaseConnection(),
\OC::$server->getEventDispatcher(),
\OC::$server->getJobList()
),
\OC::$server->getHTTPClientService(),
$keyManager,
new \OC\Security\IdentityProof\Signer(
$keyManager,
new \OC\AppFramework\Utility\TimeFactory(),
\OC::$server->getUserManager()
),
\OC::$server->getJobList()
\OC::$server->getJobList(),
$lookupServer
);
$updateLookupServer->userUpdated($user);
});
34 changes: 21 additions & 13 deletions apps/lookup_server_connector/lib/BackgroundJobs/RetryJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use OC\BackgroundJob\Job;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClientService;
use OCP\ILogger;

class RetryJob extends Job {
/** @var IClientService */
Expand All @@ -36,21 +37,28 @@ class RetryJob extends Job {
private $lookupServer = 'https://lookup.nextcloud.com/users';

/**
* @param IClientService|null $clientService
* @param IJobList|null $jobList
* @param IClientService $clientService
* @param IJobList $jobList
*/
public function __construct(IClientService $clientService = null,
IJobList $jobList = null) {
if($clientService !== null) {
$this->clientService = $clientService;
} else {
$this->clientService = \OC::$server->getHTTPClientService();
}
if($jobList !== null) {
$this->jobList = $jobList;
} else {
$this->jobList = \OC::$server->getJobList();
public function __construct(IClientService $clientService,
IJobList $jobList) {
$this->clientService = $clientService;
$this->jobList = $jobList;
}

/**
* run the job, then remove it from the jobList
*
* @param JobList $jobList
* @param ILogger $logger
*/
public function execute($jobList, ILogger $logger = null) {

if ($this->shouldRun($this->argument)) {
parent::execute($jobList, $logger);
$jobList->remove($this, $this->argument);
}

}

protected function run($argument) {
Expand Down
36 changes: 16 additions & 20 deletions apps/lookup_server_connector/lib/UpdateLookupServer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,11 @@
namespace OCA\LookupServerConnector;

use OC\Accounts\AccountManager;
use OC\Security\IdentityProof\Manager;
use OC\Security\IdentityProof\Signer;
use OCA\LookupServerConnector\BackgroundJobs\RetryJob;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClientService;
use OCP\IConfig;
use OCP\IUser;
use OCP\Security\ISecureRandom;

/**
* Class UpdateLookupServer
Expand All @@ -40,44 +37,36 @@
class UpdateLookupServer {
/** @var AccountManager */
private $accountManager;
/** @var IConfig */
private $config;
/** @var ISecureRandom */
private $secureRandom;
/** @var IClientService */
private $clientService;
/** @var Manager */
private $keyManager;
/** @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';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the default while fetching from the config instead.

Copy link
Member Author

@schiessle schiessle Mar 20, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/**
* @param AccountManager $accountManager
* @param IConfig $config
* @param ISecureRandom $secureRandom
* @param IClientService $clientService
* @param Manager $manager
* @param Signer $signer
* @param IJobList $jobList
* @param string $lookupServer if nothing is given we use the default lookup server
*/
public function __construct(AccountManager $accountManager,
IConfig $config,
ISecureRandom $secureRandom,
IClientService $clientService,
Manager $manager,
Signer $signer,
IJobList $jobList) {
IJobList $jobList,
$lookupServer = '') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$this->accountManager = $accountManager;
$this->config = $config;
$this->secureRandom = $secureRandom;
$this->clientService = $clientService;
$this->keyManager = $manager;
$this->signer = $signer;
$this->jobList = $jobList;
if ($lookupServer !== '') {
$this->lookupServer = $lookupServer;
}
$this->lookupServer = rtrim($this->lookupServer, '/');
$this->lookupServer .= '/users';
}

/**
Expand Down Expand Up @@ -113,6 +102,13 @@ protected function sendToLookupServer(IUser $user, array $publicData) {
$dataArray['website'] = isset($publicData[AccountManager::PROPERTY_WEBSITE]) ? $publicData[AccountManager::PROPERTY_WEBSITE]['value'] : '';
$dataArray['twitter'] = isset($publicData[AccountManager::PROPERTY_TWITTER]) ? $publicData[AccountManager::PROPERTY_TWITTER]['value'] : '';
$dataArray['phone'] = isset($publicData[AccountManager::PROPERTY_PHONE]) ? $publicData[AccountManager::PROPERTY_PHONE]['value'] : '';
$dataArray['twitter_signature'] = isset($publicData[AccountManager::PROPERTY_TWITTER]['signature']) ? $publicData[AccountManager::PROPERTY_TWITTER]['signature'] : '';
$dataArray['website_signature'] = isset($publicData[AccountManager::PROPERTY_WEBSITE]['signature']) ? $publicData[AccountManager::PROPERTY_WEBSITE]['signature'] : '';
$dataArray['verificationStatus'] =
[
AccountManager::PROPERTY_WEBSITE => isset($publicData[AccountManager::PROPERTY_WEBSITE]) ? $publicData[AccountManager::PROPERTY_WEBSITE]['verified'] : '',
AccountManager::PROPERTY_TWITTER => isset($publicData[AccountManager::PROPERTY_TWITTER]) ? $publicData[AccountManager::PROPERTY_TWITTER]['verified'] : '',
];
}

$dataArray = $this->signer->sign('lookupserver', $dataArray, $user);
Expand Down
5 changes: 5 additions & 0 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1515,4 +1515,9 @@
*/
'copied_sample_config' => true,

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

);
4 changes: 4 additions & 0 deletions core/img/actions/verified.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions core/img/actions/verify.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions core/img/actions/verifying.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,7 @@
'OC\\Settings\\Admin\\Sharing' => $baseDir . '/lib/private/Settings/Admin/Sharing.php',
'OC\\Settings\\Admin\\TipsTricks' => $baseDir . '/lib/private/Settings/Admin/TipsTricks.php',
'OC\\Settings\\Application' => $baseDir . '/settings/Application.php',
'OC\\Settings\\BackgroundJobs\\VerifyUserData' => $baseDir . '/settings/BackgroundJobs/VerifyUserData.php',
'OC\\Settings\\Controller\\AdminSettingsController' => $baseDir . '/settings/Controller/AdminSettingsController.php',
'OC\\Settings\\Controller\\AppSettingsController' => $baseDir . '/settings/Controller/AppSettingsController.php',
'OC\\Settings\\Controller\\AuthSettingsController' => $baseDir . '/settings/Controller/AuthSettingsController.php',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,7 @@ class ComposerStaticInit53792487c5a8370acc0b06b1a864ff4c
'OC\\Settings\\Admin\\Sharing' => __DIR__ . '/../../..' . '/lib/private/Settings/Admin/Sharing.php',
'OC\\Settings\\Admin\\TipsTricks' => __DIR__ . '/../../..' . '/lib/private/Settings/Admin/TipsTricks.php',
'OC\\Settings\\Application' => __DIR__ . '/../../..' . '/settings/Application.php',
'OC\\Settings\\BackgroundJobs\\VerifyUserData' => __DIR__ . '/../../..' . '/settings/BackgroundJobs/VerifyUserData.php',
'OC\\Settings\\Controller\\AdminSettingsController' => __DIR__ . '/../../..' . '/settings/Controller/AdminSettingsController.php',
'OC\\Settings\\Controller\\AppSettingsController' => __DIR__ . '/../../..' . '/settings/Controller/AppSettingsController.php',
'OC\\Settings\\Controller\\AuthSettingsController' => __DIR__ . '/../../..' . '/settings/Controller/AuthSettingsController.php',
Expand Down
Loading