INFENG-6103 allow no origin header requests #79
Conversation
Security InsightsThe items listed below may not capture all security relevant changes. Before providing a security review, be sure to review the entire PR for security impact. (1) Security relevant changes were detectedrest/middleware/cors.go modifiedAction Items
Questions or Comments? Reach out on HipChat: InfoSec Forum. |
Dockerfile
Outdated
| COPY . /go/src/github.com/Workiva/go-rest | ||
|
|
||
| #install glide and install dependencies | ||
| RUN go get -u github.com/Masterminds/glide |
There was a problem hiding this comment.
nit: I'd add this before the repo copy so it could potentially be cached
glide.lock
Outdated
| @@ -0,0 +1,31 @@ | |||
| hash: aa3313d279fdff7629632676678092f79f7d06aa660ea28595f6d1fe79c1a14f | |||
There was a problem hiding this comment.
As this is a library we shouldn't check in the lock file
glide.yaml
Outdated
| version: ~1.1.1 | ||
| - package: github.com/gorilla/mux | ||
| version: ~1.6.2 | ||
| - package: github.com/blakeroberts-wk/mustache |
There was a problem hiding this comment.
I'm not sure what the rules around pulling from our forks are, we should look into that
There was a problem hiding this comment.
Waiting on a Workiva fork: https://jira.atl.workiva.net/browse/COREAPPS-2112
rest/middleware/cors.go
Outdated
| originMatch := false | ||
| if origin := r.Header.Get("Origin"); checkOrigin(origin, originWhitelist) { | ||
| if checkOrigin(origin, originWhitelist) { |
There was a problem hiding this comment.
Should we add a check for the origin header being missing here, as opposed to doing the check unnecessarily?
|
@matthewsullivan-wf Would you mind weighing in on these changes? |
|
+1 security, all appears to be well |
|
+1 |
|
@Workiva/release-management-p |
blakeroberts-wk
deleted the
INFENG-6103-allow-no-origin-header-requests
branch
https://jira.atl.workiva.net/browse/INFENG-6103
What
@Workiva/product2