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

App should return 409 when private key already exists #57

Closed
ardevd opened this issue Feb 13, 2018 · 8 comments · Fixed by #150
Closed

App should return 409 when private key already exists #57

ardevd opened this issue Feb 13, 2018 · 8 comments · Fixed by #150
Assignees
Labels
2. developing Work in progress enhancement New feature or request

Comments

@ardevd
Copy link

ardevd commented Feb 13, 2018

Currently, the app will return 409 if a public key already exists. This is a very useful property, however it wont account for existing private keys. I think it should return 409 if a private key exists as well.

Also, how about returning a json field with a value indicating why 409 is returned?

@tobiasKaminsky
Copy link
Member

About which endpoint do you talk about?
Store API says:
409 conflict: if a private key for the user already exists

@ardevd
Copy link
Author

ardevd commented Feb 13, 2018

Ah, my bad. I overlooked that one. I realize now that perhaps the issue was with the Android app but I tried the following:

  1. Post a new keypair to the E2E app.
  2. Delete the public key
  3. Re-install the Android app. It will then prompt you to generate a new pass phrase. After which it posts the keypair to the server again where the E2E app seemingly ignores the private key (seemingly because it already exists) and then accepts the public key. The end result is a keypair mismatch.

Hope this made sense.

From the API point of view it makes sense that the behavior is this way. But the end-result is flawed. Im thinking the Android app should probably not upload the public key if the private key was rejected.

@ardevd
Copy link
Author

ardevd commented Feb 13, 2018

UPDATE: Looking through the Android app code I see that it does in fact check the return value when posting the private key. That means that the server seemingly returned 200 OK even though a private key already existed.

@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Feb 13, 2018

We first generate/use the public key and only if the public was stored correctly on server, we continue with private key.
Isn't this solved via nextcloud/android#2153?

@tobiasKaminsky tobiasKaminsky added the enhancement New feature or request label Feb 13, 2018
@ardevd
Copy link
Author

ardevd commented Feb 13, 2018

Yes, though in my testing the server returned 200 OK even though a private key already existed. I will do some more testing and see if I can reproduce the issue.

@georgehrke
Copy link
Member

@ardevd Were you ever able to reproduce the issue?

@georgehrke georgehrke added 0. Needs triage Pending approval or rejection. This issue is pending approval. needs info Not enough information provided labels Mar 12, 2020
@no-response
Copy link

no-response bot commented Apr 27, 2020

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

@no-response no-response bot closed this as completed Apr 27, 2020
@georgehrke georgehrke reopened this Apr 28, 2020
@georgehrke georgehrke self-assigned this Apr 28, 2020
@georgehrke georgehrke added 2. developing Work in progress and removed 0. Needs triage Pending approval or rejection. This issue is pending approval. labels Apr 28, 2020
@no-response
Copy link

no-response bot commented Apr 28, 2020

This issue has been automatically closed because there has been no response to our request for more information from the original author. With only the information that is currently in the issue, we don't have enough information to take action. Please reach out if you have or find the answers we need so that we can investigate further.

@no-response no-response bot closed this as completed Apr 28, 2020
@georgehrke georgehrke removed the needs info Not enough information provided label Apr 28, 2020
@georgehrke georgehrke reopened this Apr 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants