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.

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

After revoking a U2F second factor, revoke U2F device registration from U2F verification server #92

Merged
merged 5 commits into from Sep 30, 2015

Conversation

rjkip
Copy link
Contributor

@rjkip rjkip commented Sep 29, 2015

No description provided.

@rjkip
Copy link
Contributor Author

rjkip commented Sep 29, 2015

@DRvanR

df9

)
);

return RegistrationRevocationResult::apiError();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unrecoverable, the Middleware has already acted: the actual revocation has been executed.
Failure of the GW api is fault-tolerant: it will not cause issues in the future as a new registration will issue a different keyhandle (at least, to my understanding 😉).

Hence I think all GW revoke API errors should be properly logged, but not cause errors to be presented to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should not cause any errors to be presented to the user. Actually, I don't even use the revocation result. I still wanted to communicate the result, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

uuuuugh, somehow my mind converted all return statements to throw statements :( mi scusi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can I freely interpret this as a thumb directed away from the earth's centre?

Copy link
Contributor

Choose a reason for hiding this comment

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

why yes, yes you can 👍

ibuildings
WEB & MOBILE APP DEVELOPMENT

Daan van Renterghem
Software Engineer

phone (direct): +31 30 75 51 590
skype: dvrenterghem-ibuildings
e-mail: dvrenterghem@ibuildings.nl
phone: +31 88 00 24 000
www.ibuildings.nl

To all agreements with Ibuildings.nl BV, our general terms and conditions
http://www.ibuildings.nl/algemene_voorwaarden apply.

On Wed, Sep 30, 2015 at 11:36 AM Reinier Kip notifications@github.com
wrote:

In src/Surfnet/StepupSelfService/SelfServiceBundle/Service/U2fService.php
#92 (comment)
:

  •        return RegistrationRevocationResult::apiError();
    
  •    }
    
  •    $hasErrors = isset($result['errors']) && is_array($result['errors'])
    
  •        && $result['errors'] === array_filter($result['errors'], 'is_string');
    
  •    if ($hasErrors && $statusCode >= 400 && $statusCode < 600) {
    
  •        $this->logger->critical(
    
  •            sprintf(
    
  •                'U2F registration revocation failed; HTTP %d with errors "%s"',
    
  •                $statusCode,
    
  •                join(', ', $result['errors'])
    
  •            )
    
  •        );
    
  •        return RegistrationRevocationResult::apiError();
    

Can I freely interpret this as a thumb directed away from the earth's
centre?


Reply to this email directly or view it on GitHub
https://github.com/SURFnet/Stepup-SelfService/pull/92/files#r40775136.

rjkip added a commit that referenced this pull request Sep 30, 2015
After revoking a U2F second factor, revoke U2F device registration from U2F verification server
@rjkip rjkip merged commit a863b3b into develop Sep 30, 2015
@rjkip rjkip deleted the feature/enable-sfs branch September 30, 2015 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants