Skip to content

Commit

Permalink
fix(decode): Remove duplicate url decoder in ControllerSupport (#576)
Browse files Browse the repository at this point in the history
A call to decode the input is not necessary since Spring decodes the
path variable input by default.  See Spring's
RequestMappingHandlerMapping and UrlPathHelper.  For example, a call to
fiat `/authorize/some%2Buser@example.com` is properly decoded as
`some+user@example.com` by Spring, but then decoded again via
`ControllerSupport` -- the result is `some user@example.com`.  This can,
in certain scenarios, cause authorization lookups to return a 404.

Co-authored-by: Cameron Fieber <cameron@fieber.ca>
  • Loading branch information
mergify[bot] and cfieber committed Mar 5, 2020
1 parent 625d788 commit d91e70e
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
package com.netflix.spinnaker.fiat.controllers;

import com.netflix.spinnaker.fiat.config.UnrestrictedResourceConfig;
import java.io.UnsupportedEncodingException;
import java.net.URLDecoder;
import lombok.extern.slf4j.Slf4j;

@Slf4j
Expand All @@ -27,15 +25,9 @@ public class ControllerSupport {
static final String ANONYMOUS_USER = "anonymous";

static String convert(String in) {
try {
String decoded = URLDecoder.decode(in, "UTF-8");
if (ANONYMOUS_USER.equalsIgnoreCase(decoded)) {
return UnrestrictedResourceConfig.UNRESTRICTED_USERNAME;
}
return decoded;
} catch (UnsupportedEncodingException uee) {
log.error("Decoding exception for string " + in, uee);
if (ANONYMOUS_USER.equalsIgnoreCase(in)) {
return UnrestrictedResourceConfig.UNRESTRICTED_USERNAME;
}
return null;
return in;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,14 @@ class AuthorizeControllerSpec extends Specification {
def foo = new UserPermission().setId("foo@batman.com")

when:
controller.getUserPermission("foo%40batman.com")
controller.getUserPermission("foo@batman.com")

then:
1 * repository.get("foo@batman.com") >> Optional.empty()
thrown NotFoundException

when:
def result = controller.getUserPermission("foo%40batman.com")
def result = controller.getUserPermission("foo@batman.com")

then:
1 * repository.get("foo@batman.com") >> Optional.of(foo)
Expand Down

0 comments on commit d91e70e

Please sign in to comment.