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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ef5cf49
Save OAuth tokens with port number from token URL.
dkocher Nov 22, 2023
1f257ff
Save new tokens using OAuth Client ID as prefix. Fix #15284.
dkocher Nov 22, 2023
5a51554
Make utility methods static.
dkocher Nov 23, 2023
2a366bb
Get scheme from OAuth token URL.
dkocher Nov 23, 2023
3bf26e8
Extract field.
dkocher Nov 23, 2023
dfc52f3
Review.
dkocher Nov 23, 2023
59a2ca9
Change type.
dkocher Nov 23, 2023
edd9d3a
Add test.
dkocher Nov 29, 2023
9c2bfb8
Logging.
dkocher Nov 29, 2023
4e91997
Rename var.
dkocher Nov 29, 2023
3aba693
Only fallback to base implementation after trying all descriptors.
dkocher Nov 29, 2023
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
88 changes: 55 additions & 33 deletions core/src/main/csharp/ch/cyberduck/core/CredentialManagerPasswordStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ public override void delete(Host bookmark)
logger.info(string.Format("Delete password for bookmark {0}", bookmark));
}
var target = ToUri(bookmark);
dkocher marked this conversation as resolved.
Show resolved Hide resolved
if (!WinCredentialManager.RemoveCredentials(target.AbsoluteUri))
foreach (Uri descriptor in target)
dkocher marked this conversation as resolved.
Show resolved Hide resolved
{
base.delete(bookmark);
WinCredentialManager.RemoveCredentials(descriptor.AbsoluteUri);
}
base.delete(bookmark);
}

public override void deletePassword(string serviceName, string user)
Expand All @@ -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];
dkocher marked this conversation as resolved.
Show resolved Hide resolved
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

var cred = WinCredentialManager.GetCredentials(target.AbsoluteUri);
if (!string.IsNullOrWhiteSpace(cred.Password))
{
Expand All @@ -101,7 +102,7 @@ public override string findLoginToken(Host bookmark)
{
logger.info(string.Format("Fetching login token from keychain for {0}", bookmark));
}
var target = ToUri(bookmark);
var target = ToUri(bookmark)[0];
dkocher marked this conversation as resolved.
Show resolved Hide resolved
var cred = WinCredentialManager.GetCredentials(target.AbsoluteUri);
if (cred.Attributes is Dictionary<string, string> attrs
&& attrs.TryGetValue("Token", out var token)
Expand All @@ -119,21 +120,29 @@ public override OAuthTokens findOAuthTokens(Host bookmark)
logger.info(string.Format("Fetching OAuth tokens from keychain for {0}", bookmark));
}
var target = ToUri(bookmark);
dkocher marked this conversation as resolved.
Show resolved Hide resolved
var cred = WinCredentialManager.GetCredentials(target.AbsoluteUri);
if (cred.Attributes is Dictionary<string, string> attrs
&& attrs.TryGetValue("OAuth Access Token", out var accessToken))
foreach(Uri descriptor in target)
dkocher marked this conversation as resolved.
Show resolved Hide resolved
{
attrs.TryGetValue("OAuth Refresh Token", out var refreshToken);
attrs.TryGetValue("OIDC Id Token", out var idToken);
long expiry = default;
if (attrs.TryGetValue("OAuth Expiry", out var expiryValue))
var cred = WinCredentialManager.GetCredentials(descriptor.AbsoluteUri);
if (cred.Attributes is Dictionary<string, string> attrs)
{
long.TryParse(expiryValue, out expiry);
attrs.TryGetValue("OAuth Access Token", out var accessToken);
attrs.TryGetValue("OAuth Refresh Token", out var refreshToken);
attrs.TryGetValue("OIDC Id Token", out var idToken);
long expiry = default;
if (attrs.TryGetValue("OAuth Expiry", out var expiryValue))
{
long.TryParse(expiryValue, out expiry);
}
OAuthTokens tokens = new(accessToken, refreshToken, new(expiry), idToken);
if(tokens.validate())
{
return tokens;
}
// Continue
dkocher marked this conversation as resolved.
Show resolved Hide resolved
}
return new(accessToken, refreshToken, new(expiry), idToken);
return base.findOAuthTokens(bookmark);
}

return base.findOAuthTokens(bookmark);
return OAuthTokens.EMPTY;
}

public override string findPrivateKeyPassphrase(Host bookmark)
Expand All @@ -142,7 +151,7 @@ public override string findPrivateKeyPassphrase(Host bookmark)
{
logger.info(string.Format("Fetching private key passphrase from keychain for {0}", bookmark));
}
var target = ToUri(bookmark);
var target = ToUri(bookmark)[0];
dkocher marked this conversation as resolved.
Show resolved Hide resolved
dkocher marked this conversation as resolved.
Show resolved Hide resolved
var cred = WinCredentialManager.GetCredentials(target.AbsoluteUri);
if (cred.Attributes is Dictionary<string, string> attrs
&& attrs.TryGetValue("Private Key Passphrase", out var passphrase)
Expand Down Expand Up @@ -172,7 +181,7 @@ public override void save(Host bookmark)
{
logger.info(string.Format("Add password for bookmark {0}", bookmark));
}
var target = ToUri(bookmark);
var target = ToUri(bookmark)[0];
dkocher marked this conversation as resolved.
Show resolved Hide resolved
dkocher marked this conversation as resolved.
Show resolved Hide resolved
var credential = bookmark.getCredentials();

var winCred = new WindowsCredentialManagerCredential(
Expand Down Expand Up @@ -209,29 +218,42 @@ public override void save(Host bookmark)
}
}

private static Uri ToUri(Host bookmark)
private static List<Uri> ToUri(Host bookmark)
dkocher marked this conversation as resolved.
Show resolved Hide resolved
{
var protocol = bookmark.getProtocol();
var credentials = bookmark.getCredentials();

var targetBuilder = new UriBuilder(PreferencesFactory.get().getProperty("application.container.name"), string.Empty);
var pathBuilder = new StringBuilder();
pathBuilder.Append(protocol.getIdentifier());
if (protocol.isHostnameConfigurable() || !(protocol.isTokenConfigurable() || protocol.isOAuthConfigurable()))
List<Uri> descriptors = new();
foreach(string descriptor in ToDescriptor(bookmark))
{
pathBuilder.Append(":" + bookmark.getHostname());
if (protocol.isPortConfigurable() && !Equals(protocol.getDefaultPort(), bookmark.getPort()))
var protocol = bookmark.getProtocol();
var credentials = bookmark.getCredentials();

var targetBuilder = new UriBuilder(PreferencesFactory.get().getProperty("application.container.name"), string.Empty);
var pathBuilder = new StringBuilder();
pathBuilder.Append(descriptor);
if (protocol.isHostnameConfigurable() || !(protocol.isTokenConfigurable() || protocol.isOAuthConfigurable()))
{
pathBuilder.Append(":" + bookmark.getHostname());
if (protocol.isPortConfigurable() && !Equals(protocol.getDefaultPort(), bookmark.getPort()))
{
pathBuilder.Append(":" + bookmark.getPort());
}
}
targetBuilder.Path = pathBuilder.ToString();
if (!string.IsNullOrWhiteSpace(credentials.getUsername()))
{
pathBuilder.Append(":" + bookmark.getPort());
targetBuilder.Query = "user=" + credentials.getUsername();
}
descriptors.Add(targetBuilder.Uri);
}
targetBuilder.Path = pathBuilder.ToString();
if (!string.IsNullOrWhiteSpace(credentials.getUsername()))
return descriptors;
}

private static string[] ToDescriptor(Host bookmark)
{
if(bookmark.getProtocol().isOAuthConfigurable())
{
targetBuilder.Query = "user=" + credentials.getUsername();
return new string[] { bookmark.getProtocol().getOAuthClientId(), bookmark.getProtocol().getIdentifier() };
dkocher marked this conversation as resolved.
Show resolved Hide resolved
}

return targetBuilder.Uri;
return new string[] { bookmark.getProtocol().getIdentifier() };
dkocher marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
158 changes: 97 additions & 61 deletions core/src/main/java/ch/cyberduck/core/DefaultHostPasswordStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,37 +148,67 @@ public OAuthTokens findOAuthTokens(final Host bookmark) {
if(log.isInfoEnabled()) {
log.info(String.format("Fetching OAuth tokens from keychain for %s", bookmark));
}
final String prefix = this.getOAuthPrefix(bookmark);
final String hostname = this.getOAuthHostname(bookmark);
try {
final String expiry = this.getPassword(this.getOAuthHostname(bookmark), String.format("%s OAuth2 Token Expiry", prefix));
return (new OAuthTokens(
this.getPassword(bookmark.getProtocol().getScheme(), bookmark.getPort(), hostname,
String.format("%s OAuth2 Access Token", prefix)),
this.getPassword(bookmark.getProtocol().getScheme(), bookmark.getPort(), hostname,
String.format("%s OAuth2 Refresh Token", prefix)),
expiry != null ? Long.parseLong(expiry) : -1L,
this.getPassword(bookmark.getProtocol().getScheme(), bookmark.getPort(), hostname,
String.format("%s OIDC Id Token", prefix))));
final String[] descriptors = getOAuthPrefix(bookmark);
for(String prefix : descriptors) {
final String hostname = getOAuthHostname(bookmark);
try {
final String expiry = this.getPassword(getOAuthHostname(bookmark), String.format("%s OAuth2 Token Expiry", prefix));
final OAuthTokens tokens = new OAuthTokens(
this.getPassword(getOAuthScheme(bookmark), getOAuthPort(bookmark), hostname,
String.format("%s OAuth2 Access Token", prefix)),
this.getPassword(getOAuthScheme(bookmark), getOAuthPort(bookmark), hostname,
String.format("%s OAuth2 Refresh Token", prefix)),
expiry != null ? Long.parseLong(expiry) : -1L,
this.getPassword(getOAuthScheme(bookmark), getOAuthPort(bookmark), hostname,
String.format("%s OIDC Id Token", prefix)));
if(tokens.validate()) {
return tokens;
}
// Continue
dkocher marked this conversation as resolved.
Show resolved Hide resolved
}
catch(LocalAccessDeniedException e) {
log.warn(String.format("Failure %s searching in keychain", e));
return OAuthTokens.EMPTY;
}
}
catch(LocalAccessDeniedException e) {
log.warn(String.format("Failure %s searching in keychain", e));
return OAuthTokens.EMPTY;
return OAuthTokens.EMPTY;
}

private static Scheme getOAuthScheme(final Host bookmark) {
final URI uri = URI.create(bookmark.getProtocol().getOAuthTokenUrl());
if(null == uri.getScheme()) {
return bookmark.getProtocol().getScheme();
}
return Scheme.valueOf(uri.getScheme());
}

protected String getOAuthHostname(final Host bookmark) {
if(StringUtils.isNotBlank(URI.create(bookmark.getProtocol().getOAuthTokenUrl()).getHost())) {
return URI.create(bookmark.getProtocol().getOAuthTokenUrl()).getHost();
private static String getOAuthHostname(final Host bookmark) {
final URI uri = URI.create(bookmark.getProtocol().getOAuthTokenUrl());
if(StringUtils.isNotBlank(uri.getHost())) {
return uri.getHost();
}
return bookmark.getHostname();
}

private String getOAuthPrefix(final Host bookmark) {
if(StringUtils.isNotBlank(bookmark.getCredentials().getUsername())) {
return String.format("%s (%s)", bookmark.getProtocol().getDescription(), bookmark.getCredentials().getUsername());
private static int getOAuthPort(final Host bookmark) {
final URI uri = URI.create(bookmark.getProtocol().getOAuthTokenUrl());
if(-1 != uri.getPort()) {
return uri.getPort();
}
return bookmark.getProtocol().getDescription();
return getOAuthScheme(bookmark).getPort();
}

private static String[] getOAuthPrefix(final Host bookmark) {
dkocher marked this conversation as resolved.
Show resolved Hide resolved
chenkins marked this conversation as resolved.
Show resolved Hide resolved
if(StringUtils.isNotBlank(bookmark.getCredentials().getUsername())) {
return new String[]{
String.format("%s (%s)", bookmark.getProtocol().getOAuthClientId(), bookmark.getCredentials().getUsername()),
dkocher marked this conversation as resolved.
Show resolved Hide resolved
String.format("%s (%s)", bookmark.getProtocol().getDescription(), bookmark.getCredentials().getUsername())
};
}
return new String[]{
bookmark.getProtocol().getOAuthClientId(),
bookmark.getProtocol().getDescription()
};
}

@Override
Expand All @@ -188,6 +218,7 @@ public void save(final Host bookmark) throws LocalAccessDeniedException {
return;
}
final Credentials credentials = bookmark.getCredentials();
final Protocol protocol = bookmark.getProtocol();
if(log.isInfoEnabled()) {
log.info(String.format("Save credentials %s for bookmark %s", credentials, bookmark));
}
Expand All @@ -204,36 +235,39 @@ public void save(final Host bookmark) throws LocalAccessDeniedException {
log.warn(String.format("No password in credentials for bookmark %s", bookmark.getHostname()));
return;
}
this.addPassword(bookmark.getProtocol().getScheme(), bookmark.getPort(),
this.addPassword(protocol.getScheme(), bookmark.getPort(),
bookmark.getHostname(), credentials.getUsername(), credentials.getPassword());
}
if(credentials.isTokenAuthentication()) {
this.addPassword(bookmark.getProtocol().getScheme(), bookmark.getPort(),
this.addPassword(protocol.getScheme(), bookmark.getPort(),
bookmark.getHostname(), StringUtils.isEmpty(credentials.getUsername()) ?
bookmark.getProtocol().getTokenPlaceholder() : String.format("%s (%s)", bookmark.getProtocol().getTokenPlaceholder(), credentials.getUsername()),
protocol.getTokenPlaceholder() : String.format("%s (%s)", protocol.getTokenPlaceholder(), credentials.getUsername()),
credentials.getToken());
}
if(credentials.isOAuthAuthentication()) {
final String prefix = this.getOAuthPrefix(bookmark);
if(StringUtils.isNotBlank(credentials.getOauth().getAccessToken())) {
this.addPassword(bookmark.getProtocol().getScheme(),
bookmark.getPort(), this.getOAuthHostname(bookmark),
String.format("%s OAuth2 Access Token", prefix), credentials.getOauth().getAccessToken());
}
if(StringUtils.isNotBlank(credentials.getOauth().getRefreshToken())) {
this.addPassword(bookmark.getProtocol().getScheme(),
bookmark.getPort(), this.getOAuthHostname(bookmark),
String.format("%s OAuth2 Refresh Token", prefix), credentials.getOauth().getRefreshToken());
}
// Save expiry
if(credentials.getOauth().getExpiryInMilliseconds() != null) {
this.addPassword(this.getOAuthHostname(bookmark), String.format("%s OAuth2 Token Expiry", prefix),
String.valueOf(credentials.getOauth().getExpiryInMilliseconds()));
}
if(StringUtils.isNotBlank(credentials.getOauth().getIdToken())) {
this.addPassword(bookmark.getProtocol().getScheme(),
bookmark.getPort(), this.getOAuthHostname(bookmark),
String.format("%s OIDC Id Token", prefix), credentials.getOauth().getIdToken());
final String[] descriptors = getOAuthPrefix(bookmark);
for(String prefix : descriptors) {
dkocher marked this conversation as resolved.
Show resolved Hide resolved
if(StringUtils.isNotBlank(credentials.getOauth().getAccessToken())) {
this.addPassword(getOAuthScheme(bookmark),
getOAuthPort(bookmark), getOAuthHostname(bookmark),
String.format("%s OAuth2 Access Token", prefix), credentials.getOauth().getAccessToken());
}
if(StringUtils.isNotBlank(credentials.getOauth().getRefreshToken())) {
this.addPassword(getOAuthScheme(bookmark),
getOAuthPort(bookmark), getOAuthHostname(bookmark),
String.format("%s OAuth2 Refresh Token", prefix), credentials.getOauth().getRefreshToken());
}
// Save expiry
if(credentials.getOauth().getExpiryInMilliseconds() != null) {
this.addPassword(getOAuthHostname(bookmark), String.format("%s OAuth2 Token Expiry", prefix),
String.valueOf(credentials.getOauth().getExpiryInMilliseconds()));
}
if(StringUtils.isNotBlank(credentials.getOauth().getIdToken())) {
this.addPassword(getOAuthScheme(bookmark),
getOAuthPort(bookmark), getOAuthHostname(bookmark),
String.format("%s OIDC Id Token", prefix), credentials.getOauth().getIdToken());
}
break;
dkocher marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Expand Down Expand Up @@ -262,22 +296,24 @@ public void delete(final Host bookmark) throws LocalAccessDeniedException {
protocol.getTokenPlaceholder() : String.format("%s (%s)", protocol.getTokenPlaceholder(), credentials.getUsername()));
}
if(protocol.isOAuthConfigurable()) {
final String prefix = this.getOAuthPrefix(bookmark);
if(StringUtils.isNotBlank(credentials.getOauth().getAccessToken())) {
this.deletePassword(protocol.getScheme(), bookmark.getPort(), this.getOAuthHostname(bookmark),
String.format("%s OAuth2 Access Token", prefix));
}
if(StringUtils.isNotBlank(credentials.getOauth().getRefreshToken())) {
this.deletePassword(protocol.getScheme(), bookmark.getPort(), this.getOAuthHostname(bookmark),
String.format("%s OAuth2 Refresh Token", prefix));
}
// Save expiry
if(credentials.getOauth().getExpiryInMilliseconds() != null) {
this.deletePassword(this.getOAuthHostname(bookmark), String.format("%s OAuth2 Token Expiry", prefix));
}
if(StringUtils.isNotBlank(credentials.getOauth().getIdToken())) {
this.deletePassword(protocol.getScheme(), bookmark.getPort(), this.getOAuthHostname(bookmark),
String.format("%s OIDC Id Token", prefix));
final String[] descriptors = getOAuthPrefix(bookmark);
for(String prefix : descriptors) {
if(StringUtils.isNotBlank(credentials.getOauth().getAccessToken())) {
this.deletePassword(getOAuthScheme(bookmark), getOAuthPort(bookmark), getOAuthHostname(bookmark),
String.format("%s OAuth2 Access Token", prefix));
}
if(StringUtils.isNotBlank(credentials.getOauth().getRefreshToken())) {
this.deletePassword(getOAuthScheme(bookmark), getOAuthPort(bookmark), getOAuthHostname(bookmark),
String.format("%s OAuth2 Refresh Token", prefix));
}
// Save expiry
if(credentials.getOauth().getExpiryInMilliseconds() != null) {
this.deletePassword(getOAuthHostname(bookmark), String.format("%s OAuth2 Token Expiry", prefix));
}
if(StringUtils.isNotBlank(credentials.getOauth().getIdToken())) {
this.deletePassword(getOAuthScheme(bookmark), getOAuthPort(bookmark), getOAuthHostname(bookmark),
String.format("%s OIDC Id Token", prefix));
}
}
}
}
Expand Down