Skip to content

Commit

Permalink
Refactor setHeaders to make non case-sensitive (#739)
Browse files Browse the repository at this point in the history
* Initial commit

* WIP: No longer getting missing required header error

- now getting a different error about multiple values being returned
- still cannot find metadata file once written

* Fixed indentation

* header map with lowercase keys

* Added test for header casing conversion to lowercase

* Updated all calls to get headers to use lowercase name

* Reverted change to locustfile

* Added javadocs

* 672: Fix DomainsRegistrationTest tests

* 672: fix AuthRequestValidatorTest unit tests

* 672: Simplify the lowering of the case of the header key

---------

Co-authored-by: tjohnson7021 <[email protected]>
Co-authored-by: jorge Lopez <[email protected]>
Co-authored-by: halprin <[email protected]>
  • Loading branch information
4 people authored Dec 20, 2023
1 parent 8428ed2 commit 66cdde6
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected String retrievePublicKey() throws SecretRetrievalException {

protected String extractToken(DomainRequest request) {
logger.logDebug("Extracting token from request...");
var authHeader = Optional.ofNullable(request.getHeaders().get("Authorization")).orElse("");
var authHeader = Optional.ofNullable(request.getHeaders().get("authorization")).orElse("");
return authHeader.replace("Bearer ", "");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

/**
* Registers the available domains to the application context and their specific handlers for the
Expand Down Expand Up @@ -170,10 +172,16 @@ protected static DomainResponse authenticateRequest(DomainRequest request) {

static DomainRequest javalinContextToDomainRequest(Context ctx) {
var request = new DomainRequest();
var caseInsensitiveHeaderMap =
ctx.headerMap().entrySet().stream()
.collect(
Collectors.toMap(
entry -> entry.getKey().toLowerCase(),
Map.Entry::getValue));

request.setBody(ctx.body());
request.setUrl(ctx.url());
request.setHeaders(ctx.headerMap());
request.setHeaders(caseInsensitiveHeaderMap);
request.setPathParams(ctx.pathParamMap());

return request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,16 @@ class AuthRequestValidatorTest extends Specification{
def "extractToken happy path works"() {
given:
def token = "fake-token-here"
def header = Map.of("Authorization", "Bearer " + token)
def expected = token
def header = Map.of("authorization", "Bearer " + token)
def request = new DomainRequest()
def validator = AuthRequestValidator.getInstance()
request.setHeaders(header)
TestApplicationContext.injectRegisteredImplementations()

when:
request.setHeaders(header)
def actual = validator.extractToken(request)
def actual = AuthRequestValidator.getInstance().extractToken(request)

then:
actual == expected
actual == token
}

def "extractToken unhappy path works"() {
Expand Down Expand Up @@ -150,19 +148,21 @@ class AuthRequestValidatorTest extends Specification{
given:
def validator = AuthRequestValidator.getInstance()
def token = "fake-token-here"
def header = Map.of("Authorization", "Bearer " + token)
def header = Map.of("authorization", "Bearer " + token)
def mockEngine = Mock(JjwtEngine)
def mockCache = Mock(KeyCache)
def request = new DomainRequest()
def expected = true

request.setHeaders(header)
mockCache.get(_ as String) >> "my-fake-private-key"
mockEngine.validateToken(_ as String, _ as String)

TestApplicationContext.register(Cache, mockCache)
TestApplicationContext.register(AuthEngine, mockEngine)
TestApplicationContext.injectRegisteredImplementations()

when:
request.setHeaders(header)
mockCache.get(_ as String) >> {"my-fake-private-key"}
mockEngine.validateToken(_ as String, _ as String)
def actual = validator.isValidAuthenticatedRequest(request)

then:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class DomainsRegistrationTest extends Specification {
def bodyString = "DogCow"
def urlString = "Moof"
def headerMap = [
"Clarus": "is a DogCow"
"clarus": "is a DogCow"
]

def javalinContext = Mock(Context)
Expand Down Expand Up @@ -101,6 +101,7 @@ class DomainsRegistrationTest extends Specification {
}
def javalinContext = Mock(Context)
javalinContext.method() >> HandlerType.POST
javalinContext.headerMap() >> [:]

when:
def javalinHandler = DomainsRegistration.createHandler(rawHandler, false)
Expand Down Expand Up @@ -242,6 +243,7 @@ class DomainsRegistrationTest extends Specification {

def mockContext = Mock(Context)
mockContext.method() >> HandlerType.POST
mockContext.headerMap() >> [:]

def mockAuthValidator = Mock(AuthRequestValidator)
mockAuthValidator.isValidAuthenticatedRequest(_ as DomainRequest) >> false
Expand Down Expand Up @@ -323,6 +325,27 @@ class DomainsRegistrationTest extends Specification {
contentType == "application/x-yaml"
}

def "javalinContextToDomainRequest transforms ctx.headerMap so the keys are always lowercase"() {
given:
def headerMap = [
"testkey1": "testvalue1",
"TestKey2": "testvalue2",
"TESTKEY3": "testvalue3",
]

def javalinContext = Mock(Context)
javalinContext.headerMap() >> headerMap

when:
def domainRequest = DomainsRegistration.javalinContextToDomainRequest(javalinContext)
def transformedHeaderMap = domainRequest.getHeaders()

then:
transformedHeaderMap.get("testkey1") == "testvalue1"
transformedHeaderMap.get("testkey2") == "testvalue2"
transformedHeaderMap.get("testkey3") == "testvalue3"
}

static class Example1DomainConnector implements DomainConnector {

static def endpointCount = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ DomainResponse handleDemographics(DomainRequest request) {
DomainResponse handleOrders(DomainRequest request) {
Order<?> orders;

String submissionId = request.getHeaders().get("RecordId");
String submissionId = request.getHeaders().get("recordid");
if (submissionId == null || submissionId.isEmpty()) {
submissionId = null;
logger.logError("Missing required header or empty: RecordId");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ class EtorDomainRegistrationTest extends Specification {
def expectedStatusCode = 200

def request = new DomainRequest()
request.headers["RecordId"] = "recordId"
request.headers["recordid"] = "recordId"

def connector = new EtorDomainRegistration()
TestApplicationContext.register(EtorDomainRegistration, connector)
Expand Down Expand Up @@ -191,7 +191,7 @@ class EtorDomainRegistrationTest extends Specification {
def expectedStatusCode = 400

def request = new DomainRequest()
request.headers["RecordId"] = "recordId"
request.headers["recordid"] = "recordId"

def domainRegistration = new EtorDomainRegistration()
TestApplicationContext.register(EtorDomainRegistration, domainRegistration)
Expand Down Expand Up @@ -225,7 +225,7 @@ class EtorDomainRegistrationTest extends Specification {
def expectedStatusCode = 400

def request = new DomainRequest()
request.headers["RecordId"] = "recordId"
request.headers["recordid"] = "recordId"

def domainRegistration = new EtorDomainRegistration()
TestApplicationContext.register(EtorDomainRegistration, domainRegistration)
Expand All @@ -252,7 +252,7 @@ class EtorDomainRegistrationTest extends Specification {
given:

def request = new DomainRequest()
request.headers["RecordId"] = null //no metadata unique ID
request.headers["recordid"] = null //no metadata unique ID

def domainRegistration = new EtorDomainRegistration()
TestApplicationContext.register(EtorDomainRegistration, domainRegistration)
Expand Down Expand Up @@ -283,7 +283,7 @@ class EtorDomainRegistrationTest extends Specification {
def "handleOrders logs an error and continues the usecase like normal when the metadata unique ID is empty because we want to know when our integration with RS is broken"() {
given:
def request = new DomainRequest()
request.headers["RecordId"] = "" // empty metadata unique ID
request.headers["recordid"] = "" // empty metadata unique ID

def domainRegistration = new EtorDomainRegistration()
TestApplicationContext.register(EtorDomainRegistration, domainRegistration)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ public void setUrl(String url) {
this.url = url;
}

/**
* Returns the headers for the request. Please note that the keys (header names) are always
* lowercase.
*
* @return the headers Map for this request, with lowercase keys
*/
public Map<String, String> getHeaders() {
return headers;
}
Expand Down

0 comments on commit 66cdde6

Please sign in to comment.