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

Add support to exclude existing accounts #108

Closed
wants to merge 1 commit into from

Conversation

tobiasKaminsky
Copy link
Member

@tobiasKaminsky tobiasKaminsky commented Sep 24, 2019

Fix #106

TODO

  • check other usages of pickNewAccount

Signed-off-by: tobiasKaminsky tobias@kaminsky.me

Signed-off-by: tobiasKaminsky <tobias@kaminsky.me>
@nextcloud-android-bot
Copy link
Collaborator

Codacy

27

Lint

TypemasterPR
Warnings22
Errors00

SpotBugs (new)

Warning TypeNumber
Bad practice Warnings3
Correctness Warnings13
Internationalization Warnings2
Multithreaded correctness Warnings3
Performance Warnings3
Security Warnings4
Dodgy code Warnings8
Total36

SpotBugs (master)

Warning TypeNumber
Bad practice Warnings3
Correctness Warnings13
Internationalization Warnings2
Multithreaded correctness Warnings3
Performance Warnings3
Security Warnings4
Dodgy code Warnings8
Total36

@David-Development
Copy link
Member

I didn't have time yet to test the proposed changed. One question that came to my mind: Does this PR work when both apps have different signatures? I thought that findAccounts won't return the existing accounts if the user hasn't explicitly granted access to the accounts previously..?

@stefan-niedermann
Copy link
Member

I thought that findAccounts won't return the existing accounts if the user hasn't explicitly granted access to the accounts previously..?

This seems to be correct. I just tried it in a branch of the Notes app (given i have multiple accounts, and one of them already using in Notes), where findAccounts() will always return only the one account i already have approved.

Well, if alloweableAccounts match the result of findAccounts, the result will be, that the SSO lib assumes, there is no other account for import available and always opens the Weblogin-flow to add a completely new account.

@tobiasKaminsky
Copy link
Member Author

findAccounts() should find all Accounts, and then all existingAccounts should be removed from it.
Hm. Maybe this is again about wrong testing as I tested it with the same Android Studio and not with different signatures.

If findAccounts cannot find all Accounts by Files app (and if we do not find a way), this is useless.

@tobiasKaminsky
Copy link
Member Author

So, as far as I understand this does not work if we have different certificates, or?
Then let us postpone this until we have our own account manager in files app.

@David-Development ?

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.

Make import list filterable
4 participants