Skip to content

Commit

Permalink
fix: fixes for handling more 3xx codes and not asking for trust twice (
Browse files Browse the repository at this point in the history
…#1771)

* fix: make sure we don't ask for the same trust twice

* fix: now handling more HTTP redirect types

We were handling only a limited set of redirect types, this has now been
extended to include all common and not so common 3xx redirect codes.
This includes the somewhat strange 303 See Other where the spec isn't
completely clear but the general concensus seems to be that it forces
the next request to be a GET request.

Fixes #1769

* chore: improved trust console messages
  • Loading branch information
quintesse authored Mar 31, 2024
1 parent 826755c commit eceb5c1
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/main/java/dev/jbang/cli/Trust.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class Trust {
@CommandLine.Command(name = "add", description = "Add trust domains.")
public Integer add(
@CommandLine.Parameters(index = "0", description = "Rules for trusted sources", arity = "1..*") List<String> rules) {
TrustedSources.instance().add(rules, Settings.getTrustedSourcesFile().toFile());
TrustedSources.instance().add(rules);
return EXIT_OK;
}

Expand Down
39 changes: 35 additions & 4 deletions src/main/java/dev/jbang/net/TrustedSources.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ public class TrustedSources {
final static Pattern r127 = Pattern.compile("(?i)^127.0.0.1(:\\d+)?$");

private String[] trustedSources;
private String[] temporaryTrustedSources;

private static TrustedSources instance;

public TrustedSources(String[] trustedSources) {
TrustedSources(String[] trustedSources) {
this.trustedSources = trustedSources;
this.temporaryTrustedSources = new String[0];
}

public static TrustedSources load(Path toPath) throws IOException {
Expand All @@ -60,7 +62,7 @@ public String[] getTrustedSources() {
}

/**
* Check whether a url like https://www.microsoft.com matches the list of
* Check whether a URL like https://www.microsoft.com matches the list of
* trusted sources.
*
* - Schemes must match - There's no subdomain matching. For example
Expand All @@ -73,6 +75,15 @@ public boolean isURLTrusted(URI url) throws URISyntaxException {
return true;
}

boolean trusted = isURLTrusted(url, trustedSources) ||
isURLTrusted(url, temporaryTrustedSources) ||
// default trusted for usability and trust
url.toString().startsWith("https://github.com/jbangdev/");

return trusted;
}

private boolean isURLTrusted(URI url, String[] trustedSources) throws URISyntaxException {
final String domain = String.format("%s://%s", url.getScheme(), url.getAuthority());

for (String trustedSource : trustedSources) {
Expand Down Expand Up @@ -193,17 +204,36 @@ protected String getJSon(Collection<String> rules) {
return trustedsources;
}

public void addTemporary(String trust) {
Util.infoMsg("Trusting for this run: " + trust);
Set<String> newrules = new LinkedHashSet<>(Arrays.asList(temporaryTrustedSources));
if (newrules.add(trust)) {
temporaryTrustedSources = newrules.toArray(new String[0]);
} else {
Util.warnMsg("Already trusted source(s). No changes made.");
}
}

public void add(String trust) {
add(trust, Settings.getTrustedSourcesFile().toFile());
}

public void add(List<String> trust) {
add(trust, Settings.getTrustedSourcesFile().toFile());
}

public void add(String trust, File storage) {
add(Collections.singletonList(trust), storage);
}

public void add(List<String> trust, File storage) {

Util.infoMsg("Adding " + trust + " to " + storage);
Util.infoMsg("Trusting permanently: " + trust);

Set<String> newrules = new LinkedHashSet<>(Arrays.asList(trustedSources));

if (newrules.addAll(trust)) {
trustedSources = newrules.toArray(new String[0]);
save(newrules, storage);
} else {
Util.warnMsg("Already trusted source(s). No changes made.");
Expand All @@ -213,11 +243,12 @@ public void add(List<String> trust, File storage) {

public void remove(List<String> trust, File storage) {

Util.infoMsg("Removing " + trust + " from " + storage);
Util.infoMsg("Removing permanent trust: " + trust);

Set<String> newrules = new LinkedHashSet<>(Arrays.asList(trustedSources));

if (newrules.removeAll(trust)) {
trustedSources = newrules.toArray(new String[0]);
save(newrules, storage);
} else {
Util.warnMsg("Not found in trusted source(s). No changes made.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import java.util.ArrayList;
import java.util.List;

import dev.jbang.Settings;
import dev.jbang.cli.BaseCommand;
import dev.jbang.cli.ExitException;
import dev.jbang.net.TrustedSources;
Expand Down Expand Up @@ -60,18 +59,20 @@ public static ResourceRef fetchScriptFromUntrustedURL(String scriptURL) {
String trustOrgUrl = orgURL(trustUrl);
List<String> options = new ArrayList<>();
options.add(
"Trust once: Add no trust, just download this time (can be run multiple times while cached)");
"Trust once: Add no trust, only allow access to this URL for the duration of this run");
options.add("Trust limited url in future: " + trustUrl);
if (trustOrgUrl != null) {
options.add("Trust organization url in future: " + trustOrgUrl);
}

int result = Util.askInput(question, 30, 0, options.toArray(new String[] {}));
TrustedSources ts = TrustedSources.instance();
if (result == 2) {
ts.add(trustUrl, Settings.getTrustedSourcesFile().toFile());
if (result == 1) {
ts.addTemporary(trustUrl);
} else if (result == 2) {
ts.add(trustUrl);
} else if (result == 3) {
ts.add(trustOrgUrl, Settings.getTrustedSourcesFile().toFile());
ts.add(trustOrgUrl);
} else if (result <= 0) {
String exmsg = scriptURL
+ " is not from a trusted source and user did not confirm trust thus aborting.\n" +
Expand Down
32 changes: 21 additions & 11 deletions src/main/java/dev/jbang/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -1063,16 +1063,16 @@ private static String extractFileName(URLConnection urlConnection) throws IOExce
// extracts file name from header field
fileName = getDispositionFilename(disposition);
}
if (isBlankString(fileName)) {
// extracts file name from URL if nothing found
int p = fileURL.indexOf("?");
// Strip parameters from the URL (if any)
String simpleUrl = (p > 0) ? fileURL.substring(0, p) : fileURL;
while (simpleUrl.endsWith("/")) {
simpleUrl = simpleUrl.substring(0, simpleUrl.length() - 1);
}
fileName = simpleUrl.substring(simpleUrl.lastIndexOf("/") + 1);
}
if (isBlankString(fileName)) {
// extracts file name from URL if nothing found
int p = fileURL.indexOf("?");
// Strip parameters from the URL (if any)
String simpleUrl = (p > 0) ? fileURL.substring(0, p) : fileURL;
while (simpleUrl.endsWith("/")) {
simpleUrl = simpleUrl.substring(0, simpleUrl.length() - 1);
}
fileName = simpleUrl.substring(simpleUrl.lastIndexOf("/") + 1);
}
} else {
fileName = fileURL.substring(fileURL.lastIndexOf("/") + 1);
Expand All @@ -1087,17 +1087,27 @@ private static HttpURLConnection handleRedirects(HttpURLConnection httpConn, Con
while (true) {
httpConn.setInstanceFollowRedirects(false);
responseCode = httpConn.getResponseCode();
if (responseCode == HttpURLConnection.HTTP_MOVED_PERM ||
if (responseCode == HttpURLConnection.HTTP_MULT_CHOICE ||
responseCode == HttpURLConnection.HTTP_MOVED_PERM ||
responseCode == HttpURLConnection.HTTP_MOVED_TEMP ||
responseCode == 307 /* TEMP REDIRECT */) {
responseCode == HttpURLConnection.HTTP_SEE_OTHER ||
responseCode == 307 /* TEMP REDIRECT */ ||
responseCode == 308 /* PERM REDIRECT */) {
if (redirects++ > 8) {
throw new IOException("Too many redirects");
}
String location = httpConn.getHeaderField("Location");
if (location == null) {
throw new IOException("No 'Location' header in redirect");
}
URL url = new URL(httpConn.getURL(), location);
url = new URL(swizzleURL(url.toString()));
verboseMsg("Redirected to: " + url); // Should be debug info
httpConn = (HttpURLConnection) url.openConnection();
if (responseCode == HttpURLConnection.HTTP_SEE_OTHER) {
// This response code forces the method to GET
httpConn.setRequestMethod("GET");
}
configurator.configure(httpConn);
continue;
}
Expand Down

0 comments on commit eceb5c1

Please sign in to comment.