Project

General

Profile

Actions

Bug #19240

closed

Avoid open redirect in login process

Added by Peter Amstutz 7 months ago. Updated about 2 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
Start date:
11/04/2022
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

Add config option to allow redirect-with-token to http[s]://ipaddr:port/ where ipaddr is in one of the reserved private IP ranges ("not recommended for production")


Subtasks 1 (0 open1 closed)

Task #19639: Review 19240-check-redirectResolvedLucas Di Pentima11/04/2022

Actions
Actions #1

Updated by Peter Amstutz 7 months ago

  • Target version changed from 2022-07-20 to 2022-08-03 Sprint
Actions #2

Updated by Peter Amstutz 6 months ago

  • Target version changed from 2022-08-03 Sprint to 2022-08-17 sprint
Actions #3

Updated by Peter Amstutz 6 months ago

  • Target version changed from 2022-08-17 sprint to 2022-08-31 sprint
Actions #4

Updated by Peter Amstutz 6 months ago

  • Target version changed from 2022-08-31 sprint to 2022-09-28 sprint
Actions #5

Updated by Peter Amstutz 6 months ago

  • Target version changed from 2022-09-28 sprint to 2022-10-12 sprint
Actions #6

Updated by Peter Amstutz 4 months ago

  • Target version changed from 2022-10-12 sprint to 2022-10-26 sprint
Actions #7

Updated by Peter Amstutz 4 months ago

  • Target version changed from 2022-10-26 sprint to 2022-11-09 sprint
Actions #8

Updated by Tom Clegg 3 months ago

  • Description updated (diff)
Actions #9

Updated by Tom Clegg 3 months ago

  • Assigned To set to Tom Clegg
Actions #10

Updated by Tom Clegg 3 months ago

  • Status changed from New to In Progress
Actions #11

Updated by Tom Clegg 3 months ago

With upgrade note:

19240-check-redirect @ ff779b9f1306420462ec5898424188c88bb927a9

Actions #12

Updated by Lucas Di Pentima 3 months ago

The code LGTM. There's just one suggestion:

  • In LoginCluster federations, the admin need to list the satellite cluster's URLs, so I think we would need one of two things:
    • The easiest: Add a note about that on the upgrade notes.
    • The fancier: Make controller discover the URLs, as it doesn't make sense to avoid logins on a LoginCluster.

The "fancier" may have some edge cases, like periodically polling for URL changes, and error handling, so the "easiest" alternative is fine with me, it would just add some burden to the admins

Actions #13

Updated by Lucas Di Pentima 3 months ago

Addendum: I did read the added upgrade notes, but I think it would be nice to explicitly say that LoginCluster federations also need this config update.

Actions #14

Updated by Tom Clegg 3 months ago

Good point. Added a bit to call out the federation case specifically.

Agree automatically recognizing remote clusters' URLs would be better, but I think we can call that an additional feature...? (We're already relying on adding these manually. I updated the relevant bits of the federation docs to a) remind to add wb2 as well as wb1 and b) use proper example domains like cluster2.example instead of cluster2.com)

19240-check-redirect @ 710dc7f830f65232389cf191028edfdfe4cefe77

Actions #15

Updated by Lucas Di Pentima 3 months ago

LGTM, thanks!

Actions #16

Updated by Tom Clegg 3 months ago

  • Status changed from In Progress to Resolved
Actions #17

Updated by Peter Amstutz about 2 months ago

  • Release set to 47
Actions

Also available in: Atom PDF