Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/pomerium: strip port from authentication handler #106

Conversation

desimone
Copy link
Contributor

@desimone desimone commented May 4, 2019

Go strips the port from the host only when it is trying to locate a http handler. It does not strip the port when the user adds a handler for a host that includes a port number. As a result, trying to add handlers for hosts that include the port no longer works. To fix a bug where authenticate handler's were not being found, we explicitly strip the auth host handler of any port.

Closes #104

See also:

Checklist:

  • unit tests added
  • related issues referenced
  • ready for review

@codecov
Copy link

codecov bot commented May 4, 2019

Codecov Report

Merging #106 into master will increase coverage by 0.07%.
The diff coverage is 87.5%.

@@            Coverage Diff             @@
##           master     #106      +/-   ##
==========================================
+ Coverage   79.47%   79.55%   +0.07%     
==========================================
  Files          31       32       +1     
  Lines        1832     1839       +7     
==========================================
+ Hits         1456     1463       +7     
  Misses        307      307              
  Partials       69       69

@desimone desimone force-pushed the bug/104-fix-port-not-being-striped-from-handler branch from a6a02c1 to fc9aad2 Compare May 4, 2019 03:11
@desimone desimone changed the title internal/httputil: add strip port function cmd/pomerium: strip port from authentication handler May 4, 2019
@desimone desimone merged commit 286aad3 into pomerium:master May 4, 2019
@desimone desimone deleted the bug/104-fix-port-not-being-striped-from-handler branch May 4, 2019 03:21
@desimone desimone added this to the v0.0.5 milestone May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

authenticate: handlers return a 404 not found if the route contains a port
1 participant