Use X-Forwarded-Prefix if present to handle PathPrefixStrip traefik rules

[seuf]

When Traefik is configured with PathPrefixStrip rules, the X-Forwarded-Uri is equal to / instead of the prefix. This fix use the X-Forwarded-Prefix Header when present to allow correct redirection and rules matching.

[thomseddon]

Thank you for the pull request!

This feature was also proposed against v0 in #39 which I was planning to merge into this branch.

There's a few changes that will be needed here:

• My understanding of PathPrefixStrip is that X-Forwarded-Prefix will contain the stripped part, not the entire path. As such, the full path is the concatenation of X-Forwarded-Prefix and X-Forwarded-Uri (as done here: https://github.com/thomseddon/traefik-forward-auth/pull/39/files#diff-dc18d370e42b9d70e5569705202625aeR211)
• Would you be willing to also add support for the X-Replaced-Path case?
• Please could you add some tests to verify the intended behaviour

[seuf]

Hi @thomseddon do you know when you will have time to review / merge this ? Thanks.

[thomseddon]

I'm hoping to have a look at this and a few others next week :)

[seuf]

Hello!

Sorry for the delay in reviewing this, I've just had a look through, the main thing I'm not sure about is the modification the r.URL in the Server RootHandler - what was you're thinking behind this?

The idea is that the rules are based on the requests. So the RootHandler rewrite the incoming request according to the Headers X-Forwarded-Uri and X-Replaced-Path if presents.

[thomseddon]

Right, I thought so.

I just wondered if it was more logical to write rules based on the original request, or based on the modified request - do you have a use case where you needed the former?

[seuf]

I've tried to use a PathPrefixStrip rule in traefik ingress and the returned uri was / instead of the wanted path. If we handle the request path in the root handler, the logic of rules and redirects after login are simpler.

Le lun. 8 juil. 2019 18:27, Thom Seddon [email protected] a écrit :

Right, I thought so.

I just wondered if it was more logical to write rules based on the original request, or based on the modified request - do you have a use case where you needed the former?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/thomseddon/traefik-forward-auth/pull/49?email_source=notifications&email_token=AAKGIJ3X2RM6BLAVCXTOQSTP6NTHRA5CNFSM4HW5BOC2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZNUABI#issuecomment-509296645, or mute the thread https://github.com/notifications/unsubscribe-auth/AAKGIJ7NI5AXYMF4ROTH2F3P6NTHRANCNFSM4HW5BOCQ .

[seuf]

Do I need to change something ? We really need this feature to be merged in upstream

[seuf]

@thomseddon Do you plan to merge this ? Do you need more changes ?

[seuf]

Sorry, I didn't see your change request. I've added the fix you suggested.

[sriramsv]

HI @seuf and @thomseddon, I stumbled upon this issue as well and wanted to see this PR merged asap as well as I am also using PathPrefixStrip routes in my production setup. Please take a look into this asap.

[seuf]

I've rebased over master to avoid conflicts

[seuf]

ping ?

[seuf]

ping ?

[seuf]

@thomseddon I've pushed an update on the function to check the X-Forwared-Prefix first as you suggested. is it ok for you ?

[seuf]

+1 ?

[dicksnel]

This would be great so StripPrefix can be used.

[dhirschfeld]

I'm glad I found this issue - I had no idea what I was doing wrong! 😖

If this works, it would be great if it could be merged. As they say, perfect is the enemy of shipped...

[dhirschfeld]

I can confirm that running this PR works! :tada:

[seuf]

+1 ?