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

Save OAuth tokens with port number, scheme and hostname from token URL. #15393

Merged
merged 11 commits into from
Dec 4, 2023
Merged

Save OAuth tokens with port number, scheme and hostname from token URL. #15393

merged 11 commits into from
Dec 4, 2023

Conversation

dkocher
Copy link
Contributor

@dkocher dkocher commented Nov 22, 2023 •
edited
Loading

@dkocher dkocher requested a review from a team as a code owner November 22, 2023 20:09
@dkocher dkocher added this to the 8.7.2 milestone Nov 22, 2023
@dkocher dkocher linked an issue Nov 22, 2023 that may be closed by this pull request
Shared OAuth refresh token not accepted #15284
Closed
@dkocher dkocher marked this pull request as draft November 22, 2023 20:36
@dkocher dkocher marked this pull request as ready for review November 23, 2023 08:27
@dkocher dkocher marked this pull request as draft November 23, 2023 08:28
@dkocher dkocher marked this pull request as ready for review November 23, 2023 10:00
AliveDevil
AliveDevil approved these changes Nov 23, 2023
View reviewed changes
Copy link
Contributor

@AliveDevil AliveDevil left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@chenkins chenkins changed the title Save OAuth tokens with port number from token URL. Save OAuth tokens with port number, scheme and hostname from token URL. Nov 27, 2023
chenkins
chenkins reviewed Nov 28, 2023
View reviewed changes
core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java Outdated Show resolved Hide resolved
core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java Outdated Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Outdated Show resolved Hide resolved
core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java Show resolved Hide resolved
core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
@AliveDevil AliveDevil removed their request for review November 28, 2023 18:15
chenkins
chenkins reviewed Nov 28, 2023
View reviewed changes
Copy link
Contributor

@chenkins chenkins left a comment

Choose a reason for hiding this comment

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

Left my comments/suggestions.

chenkins
chenkins approved these changes Nov 29, 2023
View reviewed changes
Copy link
Contributor

@chenkins chenkins left a comment

Choose a reason for hiding this comment

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

LGTM with suggestions.

core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs
@@ -86,7 +87,7 @@ public override void deletePassword(Scheme scheme, int port, string hostName, st

public override string findLoginPassword(Host bookmark)
{
var target = ToUri(bookmark);
var target = ToUri(bookmark)[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
var target = ToUri(bookmark)[0];
var target = ToUri(bookmark)[0]; // non-OAuth case

core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs Show resolved Hide resolved
@AliveDevil AliveDevil mentioned this pull request Nov 29, 2023
Open
@AliveDevil
Copy link
Contributor

Requires instructing support and amending Docs.
iterate-ch/docs#453

@dkocher dkocher merged commit 9891963 into master Dec 4, 2023
5 checks passed
@dkocher dkocher deleted the bugfix/GH-15113-port branch December 4, 2023 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chenkins chenkins chenkins approved these changes

@AliveDevil AliveDevil AliveDevil approved these changes

@ylangisc ylangisc ylangisc approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
8.7.2
4 participants
@dkocher @AliveDevil @ylangisc @chenkins