Project

General

Profile

Actions

Bug #19240

closed

Avoid open redirect in login process

Added by Peter Amstutz almost 2 years ago. Updated over 1 year ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
API
Target version:
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/2022Actions
Actions #1

Updated by Peter Amstutz almost 2 years ago

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

Updated by Peter Amstutz almost 2 years ago

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

Updated by Peter Amstutz over 1 year ago

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

Updated by Peter Amstutz over 1 year ago

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

Updated by Peter Amstutz over 1 year ago

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

Updated by Peter Amstutz over 1 year ago

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

Updated by Peter Amstutz over 1 year ago

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

Updated by Tom Clegg over 1 year ago

  • Description updated (diff)
Actions #9

Updated by Tom Clegg over 1 year ago

  • Assigned To set to Tom Clegg
Actions #10

Updated by Tom Clegg over 1 year ago

  • Status changed from New to In Progress
Actions #11

Updated by Tom Clegg over 1 year ago

With upgrade note:

19240-check-redirect @ ff779b9f1306420462ec5898424188c88bb927a9

Actions #12

Updated by Lucas Di Pentima over 1 year 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 over 1 year 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 over 1 year 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 over 1 year ago

LGTM, thanks!

Actions #16

Updated by Tom Clegg over 1 year ago

  • Status changed from In Progress to Resolved
Actions #17

Updated by Peter Amstutz over 1 year ago

  • Release set to 47
Actions

Also available in: Atom PDF