-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds proxy method to call API using session #10201
Conversation
@@ -111,7 +111,7 @@ public function showInContext() | |||
$controllerName = Common::getRequestVar('moduleToLoad'); | |||
$actionName = Common::getRequestVar('actionToLoad', 'index'); | |||
|
|||
if($controllerName == 'API') { | |||
if($controllerName == 'API' || ($controllerName == 'Proxy' && $actionName == 'callApi')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move whole this condition to separate method eg. isSupportedRequest
or isAllowedRequest
?
@sgiehl 👍 (I left a few comments) |
Thx for the comments. I'll check those the comming days |
…ntication; use proxy method for scheduled reports download links
…o avoid exposing of token_auth
@andrzejewsky I've adjusted most of the stuff according to your feedback. @tsteur @mattab anyone time for a quick look at the PR, so we can merge it? |
I'm quite busy :( don't think I'll have time for it soon as it will take some time re security etc |
Maybe some tests could be added? Or is tested via the export UI tests already maybe? |
Review
|
fyi reasons for not merging were:
|
New proxy method is used for download links in email reports and datatables, to avoid exposing the token_auth in direct API urls.
fixes #10185
fixes #10147