Bug 53534 - Fix Auth exploit that was taken over from Kelvin
Summary: Fix Auth exploit that was taken over from Kelvin
Status: VERIFIED FIXED
Alias: None
Product: Components
Classification: Unclassified
Component: ucsschool-apis
Version: unspecified
Hardware: Other Mac OS X 10.1
: P5 normal
Target Milestone: ---
Assignee: Ole Schwiegert
QA Contact: UCS@school maintainers
URL:
Keywords:
: 54075 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-06-30 21:11 CEST by Ole Schwiegert
Modified: 2021-11-19 02:39 CET (History)
2 users (show)

See Also:
What kind of report is it?: ---
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional):
Customer ID:
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ole Schwiegert univentionstaff 2021-06-30 21:11:24 CEST
The security breach that was found in the Kelvin API found its way into the ucsschool-apis App as well, since it was derived from Kelvin.

It needs to be fixed here as well. This is not a big problem since the App was never released yet.
Comment 1 Ole Schwiegert univentionstaff 2021-06-30 21:45:22 CEST
Fixed in oschwieg/53534
Comment 2 Joerg Baach univentionstaff 2021-07-01 11:18:17 CEST
QA: nok

confirm bug
===========

I try to do QA with buildingslogin, the wrong tokens come from a kelvin server

- get/build wrong token
  - Fetch a token from kelvin server 
- access protected resource on http://localhost:8080/ucsschool/apis/bildungslogin/user/demo_student'' \
- see success
- => {"id":"demo_student","licenses":["foo-123","bar-456"],"context":{"DEMOSCHOOL":{"classes":["Democlass"],"roles":["student"]}}}
  
Fix on branch
=============

- build new image? done
- adapt plugin # THIS FEELS WRONG 
- get/build wrong token
  - fetch from kelvin
- access protected resource, see fail  
  = =>{"detail":"Could not validate credentials"}
- get/build correct token
- access protected resource
- =>{"id":"demo_student","licenses":["foo-123","bar-456"],"context":{"DEMOSCHOOL":{"classes":["Democlass"],"roles":["student"]}}}


It works in principle, but maybe it would be better if the plugins wouldn't need adaption. Suggestion: rename oauth2_scheme->something_else, get_token -> oauth2_schema
Comment 3 Ole Schwiegert univentionstaff 2021-07-01 11:22:05 CEST
I would argument the other way around:

The ucsschool-apis App is not released yet, hence also the plugins are not released yet and completely in development.

If we implement your suggested change we get a difference between the Kelvin and ucsschool-apis code.

If we someday merge the shared code for auth (which is still intended) we get the discrepancy and need to adapt one or the other then.
Comment 4 Joerg Baach univentionstaff 2021-07-01 14:49:48 CEST
Your argument about the discrepancy between kelvin and ucsschool-apis wins.

QA: ok
Comment 5 Joerg Baach univentionstaff 2021-07-01 14:50:51 CEST
QA was ok, I ment to set it to verified 
-> verified fixed
Comment 6 Ole Schwiegert univentionstaff 2021-11-16 15:50:30 CET
*** Bug 54075 has been marked as a duplicate of this bug. ***