foreman icon indicating copy to clipboard operation
foreman copied to clipboard

Fixes #35473 - Add extlogin API endpoint

Open ofedoren opened this issue 1 year ago • 6 comments

/users/extlogin endpoint is designed for UI interaction, thus using this endpoint to create a session to be used via API will fail with "Can't verify CSRF token authenticity" for any method except GET. We need to have a separate endpoint to create a proper session to be used via API.

  • [ ] Requires https://github.com/theforeman/puppet-foreman/pull/1083
  • [ ] Required by https://github.com/theforeman/hammer-cli-foreman/pull/605

I'd like to have it CPed into 3.3, but not sure if it's feasible...

I've tested these (and deps) patches on production setup via:

  • curl -k -c cookies.txt -u : --negotiate https://foreman.ofedoren.example.com/api/users/extlogin [OK]
  • curl -b cookies.txt -H "Accept:application/json,version=2" -H "Content-Type:application/json" -X POST -d '{"architecture": { "name": "arch1" }}' -k https://foreman.ofedoren.example.com/api/architectures [OK]
  • hammer auth login negotiate [OK]
  • hammer architecture create --name arch2 [OK]

P.S. This is a suggested solution. One of the possible solutions could be introduced here only. We would need to somehow make sure that https://github.com/theforeman/foreman/blob/develop/app/controllers/concerns/foreman/controller/authentication.rb#L88-L92 works with hitting /users/extlogin via API.

ofedoren avatar Sep 05 '22 16:09 ofedoren

Issues: #35473

theforeman-bot avatar Sep 05 '22 16:09 theforeman-bot

I have a conceptual question. We have CSRF protection on the normal extlogin page, so that nobody can build a fake and abuse that (as the fake won't have the right token). When we now introduce a CSRF-less api/extlogin, wouldn't the attacker just use that as an entry point and be happy? If the answer is yes, then we could also just disable CSRF on extlogin? Or am I missing some integral part of the whole thing?

evgeni avatar Sep 07 '22 11:09 evgeni

Isn't the csrf protection there to limit the damage an attacker can do if they manage to hijack an already existing session?

adamruzicka avatar Sep 07 '22 12:09 adamruzicka

Yes, but wouldn't hitting /api/users/extlogin in the current setup with an existing session "upgrade" said session to a CSRF-less one without further user interaction?

evgeni avatar Sep 07 '22 12:09 evgeni

Okay, no, one can't upgrade a session to an API one. I shut up ;)

evgeni avatar Sep 07 '22 13:09 evgeni

Not sure what's up with the tests [test integration] [test katello] [test unit]

adamruzicka avatar Sep 14 '22 11:09 adamruzicka

@ekohl should we hold this until the puppet module is sorted out?

Also kicking off [test integration] once more

adamruzicka avatar Oct 05 '22 10:10 adamruzicka

LGTM and I don't think it needs to wait for the Puppet changes.

It won't work without them in prod setups, but it also won't break and all we discussed in the Puppet PR was style, not the actual routes or anything that would influence the PR here.

evgeni avatar Oct 28 '22 06:10 evgeni

Thank you @ofedoren & @evgeni !

adamruzicka avatar Oct 31 '22 10:10 adamruzicka