Remove set_user_info from URI (#8738)

I have added this in #8591, but I have realised it may not be a good idea to have it, so I am removing that particular change.
This commit is contained in:
Radosław Waśko 2024-01-15 18:35:17 +01:00 committed by GitHub
parent f34abeda0c
commit 5b70ff25f7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 20 additions and 131 deletions

View File

@ -27,7 +27,6 @@ polyglot java import org.graalvm.collections.Pair as Java_Pair
polyglot java import org.enso.base.enso_cloud.EnsoSecretAccessDenied
polyglot java import org.enso.base.net.URITransformer
polyglot java import org.enso.base.net.URIWithSecrets
polyglot java import org.enso.base.net.UserInfoWithSecrets
## Represents a Uniform Resource Identifier (URI) reference.
type URI
@ -49,7 +48,7 @@ type URI
example_parse = URI.parse "http://example.com"
parse : Text -> URI ! Syntax_Error
parse uri:Text =
Panic.catch URISyntaxException (URI.Value (Java_URI.new uri) [] Nothing) caught_panic->
Panic.catch URISyntaxException (URI.Value (Java_URI.new uri) []) caught_panic->
message = caught_panic.payload.getMessage
truncated = if message.is_nothing || message.length > 100 then "Invalid URI '" + uri.to_display_text + "'" else
"URI syntax error: " + message
@ -147,7 +146,7 @@ type URI
segments = if segment == "" then ["", ""] else
segment.split "/"
result = URI.Value (URITransformer.extendPath self.internal_uri segments) self.additional_query_parameters self.user_info_override
result = URI.Value (URITransformer.extendPath self.internal_uri segments) self.additional_query_parameters
Problem_Behavior.Report_Warning.attach_problems_before (has_question_mark+has_hash) result
## GROUP Metadata
@ -171,24 +170,13 @@ type URI
- value: The value of the query parameter.
add_query_argument : Text -> Text | Enso_Secret -> URI
add_query_argument self key:Text value:(Text | Enso_Secret) =
URI.Value self.internal_uri self.additional_query_parameters+[Pair.new key value] self.user_info_override
URI.Value self.internal_uri self.additional_query_parameters+[Pair.new key value]
## GROUP Calculations
Removes any query parameters within the URI.
reset_query_arguments : URI
reset_query_arguments self = handle_resolve_errors <|
URI.Value (URITransformer.removeQueryParameters self.internal_uri) [] self.user_info_override
## GROUP Calculations
Sets the user info for the URI.
Arguments:
- username: The username.
- password: The password.
set_user_info : Text | Enso_Secret -> Text | Enso_Secret -> URI
set_user_info self (username : Text | Enso_Secret) (password : Text | Enso_Secret) =
if username.is_a Text && username.contains ":" then Error.throw (Illegal_Argument.Error "The username cannot contain a ':' character.") else
URI.Value self.internal_uri self.additional_query_parameters (User_Info.Value username password)
URI.Value (URITransformer.removeQueryParameters self.internal_uri) []
## Get the fragment part of this URI.
@ -259,9 +247,7 @@ type URI
Arguments:
- internal_builder: A Java URI that contains parsed URI data.
- additional_query_parameters: A list of query parameters to add to the URI.
- user_info_override: The user and password for the authority part of the URL, if provided.
If the `internal_uri` also contains user info, the one provided here will override it.
Value (internal_uri : Java_URI) (additional_query_parameters : Vector (Pair Text (Text | Enso_Secret))) (user_info_override : User_Info | Nothing)
Value (internal_uri : Java_URI) (additional_query_parameters : Vector (Pair Text (Text | Enso_Secret)))
## PRIVATE
Convert to a JavaScript Object representing this URI.
@ -286,10 +272,7 @@ type URI
to_java_representation self =
parameters = self.additional_query_parameters.map p->
Java_Pair.create p.first (Cloud_Utils.as_hideable_value p.second)
java_user_info = self.user_info_override.if_not_nothing <|
self.user_info_override.to_java
URIWithSecrets.new self.internal_uri parameters java_user_info
URIWithSecrets.new self.internal_uri parameters
## PRIVATE
Convert this to a raw Java URI.
@ -300,18 +283,6 @@ type URI
## PRIVATE
URI.from (that:Text) = URI.parse that
## PRIVATE
type User_Info
## PRIVATE
Value (username : Text | Enso_Secret) (password : Text | Enso_Secret)
## PRIVATE
to_java : UserInfoWithSecrets
to_java self =
username = Cloud_Utils.as_hideable_value self.username
password = Cloud_Utils.as_hideable_value self.password
UserInfoWithSecrets.new username password
## PRIVATE
handle_resolve_errors ~action =
handle_access_denied _ =
@ -330,7 +301,7 @@ type URI_Comparator
eq = case java_uri_x.is_error || java_uri_y.is_error of
# If the URIs contain secrets, they are considered equal only if they yield the same text and have the same internal structure.
True -> if x.to_text != y.to_text then False else
(x.additional_query_parameters == y.additional_query_parameters) && (x.user_info_override == y.user_info_override)
(x.additional_query_parameters == y.additional_query_parameters)
# If the URIs do not contain secrets, they are considered equal if the effective URI they denote is the same.
False -> java_uri_x == java_uri_y
if eq then Ordering.Equal else Nothing

View File

@ -52,13 +52,7 @@ public class EnsoSecretHelper {
uri.queryParameters().stream()
.map(p -> Pair.create(p.getLeft(), resolveValue(p.getRight())))
.toList();
Pair<String, String> resolvedUserInfo =
uri.userInfo() == null
? null
: Pair.create(
resolveValue(uri.userInfo().username()), resolveValue(uri.userInfo().password()));
URISchematic resolvedSchematic =
new URISchematic(uri.baseUri(), resolvedQueryParameters, resolvedUserInfo);
URISchematic resolvedSchematic = new URISchematic(uri.baseUri(), resolvedQueryParameters);
return resolvedSchematic.build();
} catch (URISyntaxException e) {
// Here we don't display the message of the exception to avoid risking it may leak any

View File

@ -14,23 +14,10 @@ import org.graalvm.collections.Pair;
*
* <p>This is the common entry point for building a URI with or without secrets.
*/
public record URISchematic(
URI baseUri, List<Pair<String, String>> queryParameters, Pair<String, String> userInfo) {
public record URISchematic(URI baseUri, List<Pair<String, String>> queryParameters) {
public URI build() throws URISyntaxException {
StringBuilder authorityBuilder = new StringBuilder();
if (userInfo != null) {
var username = userInfo.getLeft();
var password = userInfo.getRight();
if (username.contains(":")) {
throw new IllegalArgumentException("Username within an URI cannot contain ':'.");
}
authorityBuilder
.append(URITransformer.encode(username))
.append(":")
.append(URITransformer.encode(password))
.append("@");
} else if (baseUri.getRawUserInfo() != null) {
if (baseUri.getRawUserInfo() != null) {
authorityBuilder.append(baseUri.getRawUserInfo()).append("@");
}

View File

@ -10,21 +10,16 @@ import org.graalvm.collections.Pair;
* A structure representing a URI that contains parts which may need to be updated once data from
* secrets is resolved.
*
* <p>The query parameters and user info are stored separately, because they may contain secrets and
* will only be resolved to plain values within {@link org.enso.base.enso_cloud.EnsoSecretHelper}.
* <p>The query parameters are stored separately, because they may contain secrets and will only be
* resolved to plain values within {@link org.enso.base.enso_cloud.EnsoSecretHelper}.
*/
public record URIWithSecrets(
URI baseUri, List<Pair<String, HideableValue>> queryParameters, UserInfoWithSecrets userInfo) {
public record URIWithSecrets(URI baseUri, List<Pair<String, HideableValue>> queryParameters) {
/** Creates a schematic that does not disclose secret values and can be returned to the user. */
public URISchematic makeSchematicForRender() {
List<Pair<String, String>> renderedParameters =
queryParameters.stream().map(p -> Pair.create(p.getLeft(), p.getRight().render())).toList();
Pair<String, String> renderedUserInfo =
userInfo == null
? null
: Pair.create(userInfo.username().render(), userInfo.password().render());
return new URISchematic(baseUri, renderedParameters, renderedUserInfo);
return new URISchematic(baseUri, renderedParameters);
}
public URI render() {
@ -48,10 +43,6 @@ public record URIWithSecrets(
}
public boolean containsSecrets() {
if (userInfo != null && (userInfo.username().isSecret() || userInfo.password().isSecret())) {
return true;
}
return queryParameters.stream().anyMatch(p -> p.getRight().isSecret());
}
@ -60,11 +51,7 @@ public record URIWithSecrets(
queryParameters.stream()
.map(p -> Pair.create(p.getLeft(), p.getRight().safeResolve()))
.toList();
Pair<String, String> resolvedUserInfo =
userInfo == null
? null
: Pair.create(userInfo.username().safeResolve(), userInfo.password().safeResolve());
return new URISchematic(baseUri, resolvedParameters, resolvedUserInfo);
return new URISchematic(baseUri, resolvedParameters);
}
public String getScheme() {
@ -74,7 +61,7 @@ public record URIWithSecrets(
private URI forAuthorityPart() {
// We can ignore secrets in the query part, because they are not used for resolving the
// authority.
return new URIWithSecrets(baseUri, List.of(), userInfo).safeResolve();
return new URIWithSecrets(baseUri, List.of()).safeResolve();
}
public String getUserInfo() {
@ -111,9 +98,7 @@ public record URIWithSecrets(
}
private URI forQueryPart() {
// We can ignore secrets in the authority part, because they are not used for resolving the
// query.
return new URIWithSecrets(baseUri, queryParameters, null).safeResolve();
return safeResolve();
}
public String getQuery() {

View File

@ -1,5 +0,0 @@
package org.enso.base.net;
import org.enso.base.enso_cloud.HideableValue;
public record UserInfoWithSecrets(HideableValue username, HideableValue password) {}

View File

@ -167,49 +167,18 @@ spec =
. add_query_argument "a" "d"
decode_query_params uri.fetch . should_equal [["a", "b"], ["a", "c"], ["a", "d"]]
Test.specify "should allow to set user info for the authority part of the URI" <|
uri1 = URI.parse "http://example.com"
uri2 = uri1.set_user_info "my_user" "my_password"
uri2.to_text . should_equal "http://my_user:my_password@example.com"
uri2.user_info . should_equal "my_user:my_password"
uri2.authority . should_equal "my_user:my_password@example.com"
uri3 = URI.parse "https://start:pass@example.org:1234/path"
uri4 = uri3.set_user_info "new_user" "new_password"
uri4.to_text . should_equal "https://new_user:new_password@example.org:1234/path"
uri4.user_info . should_equal "new_user:new_password"
uri4.raw_user_info . should_equal "new_user:new_password"
uri4.authority . should_equal "new_user:new_password@example.org:1234"
uri4.raw_authority . should_equal "new_user:new_password@example.org:1234"
uri4.path . should_equal "/path"
Test.specify "should allow non-standard characters in the user info" <|
uri1 = URI.parse "http://X:Y@example.com"
uri2 = uri1.set_user_info "ąęć@x" "ś🚧: @"
uri2.to_text . should_equal "http://%C4%85%C4%99%C4%87%40x:%C5%9B%F0%9F%9A%A7%3A%20%40@example.com"
uri2.user_info . should_equal "ąęć@x:ś🚧: @"
uri2.raw_user_info . should_equal "%C4%85%C4%99%C4%87%40x:%C5%9B%F0%9F%9A%A7%3A%20%40"
uri2.authority . should_equal "ąęć@x:ś🚧: @@example.com"
uri2.raw_authority . should_equal "%C4%85%C4%99%C4%87%40x:%C5%9B%F0%9F%9A%A7%3A%20%40@example.com"
Test.specify "will not allow to set username containing a colon" <|
uri1 = URI.parse "http://example.com"
uri2 = uri1.set_user_info "a:b" "c"
uri2.should_fail_with Illegal_Argument
Test.specify "should allow to get properties of a URI with added query arguments and user info" <|
base_uri = URI.parse "https://example.com/path?a=b#FRAG"
uri1 = base_uri . add_query_argument "c" "d" . set_user_info "U" "P" . add_query_argument "e" "f"
uri1 = base_uri . add_query_argument "c" "d" . add_query_argument "e" "f"
uri1.should_be_a URI
uri1.scheme.should_equal "https"
uri1.user_info.should_equal "U:P"
uri1.user_info.should_equal Nothing
uri1.host.should_equal "example.com"
uri1.path.should_equal "/path"
uri1.query.should_equal "a=b&c=d&e=f"
uri1.fragment.should_equal "FRAG"
uri1.should_equal (URI.parse "https://U:P@example.com/path?a=b&c=d&e=f#FRAG")
uri1.should_equal (URI.parse "https://example.com/path?a=b&c=d&e=f#FRAG")
Test.specify "should allow the / syntax for extending the path on an URI" <|
uri0 = URI.parse "https://example.com"
@ -348,18 +317,6 @@ spec =
response = cloud_setup.httpbin_secure_client.request (Request.get uri)
decode_query_params response.decode_as_json . should_equal [["arg1", "My Very Secret Value"], ["arg2", "plain value"], ["arg3", s2]]
Test.specify "should correctly handle user/password set as secret" pending=cloud_setup.pending <|
secret1 = Enso_Secret.create "my_test_secret-uri-6" "My Password"
secret1.should_succeed
Panic.with_finalizer secret1.delete <|
uri1 = URI.from "https://example.com/"
. set_user_info "my_user" secret1
uri1.to_text . should_equal ("https://my_user:__SECRET__@example.com/")
uri2 = URI.from "https://example.com/"
. set_user_info secret1 "pass"
uri2.to_text . should_equal ("https://__SECRET__:pass@example.com/")
Test.specify "does not allow secrets in non-https requests" pending=cloud_setup.pending <|
secret1 = Enso_Secret.create "my_test_secret-uri-8" "My Value"
secret1.should_succeed