From b0fe972b3754cd76646ca7d0b3fb78ea91169af4 Mon Sep 17 00:00:00 2001 From: Benjamin Schanzel Date: Tue, 25 Mar 2025 13:24:41 +0100 Subject: [PATCH] web: Pass auth header to log endpoint if serving from the same origin If we serve log files from the same origin as the API, pass an authorization header with the log file request if available. This is to make sure we don't have to do authentication for the log file requests again if the same reverse proxy is handling the requests and authorization. This must be enabled explictly by setting a new zuul-web config flag `web.auth_log_file_requests` to true. Change-Id: I32139671db194c1407146911f677ad2516159e11 --- doc/source/configuration.rst | 9 +++++ tests/unit/test_web.py | 2 ++ web/src/actions/build.js | 11 +++--- web/src/actions/logfile.js | 5 +-- web/src/api.js | 17 +++++++++ web/src/api.test.js | 69 +++++++++++++++++++++++++++++++++++- zuul/web/__init__.py | 10 ++++++ 7 files changed, 115 insertions(+), 8 deletions(-) diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 9df286f005..0219804b24 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -1006,6 +1006,15 @@ sections of ``zuul.conf`` are used by the web server: If this is used the finger gateways should be configured accordingly. + .. attr:: auth_log_file_requests + :default: false + + If set to true, the JavaScript web client will pass an Authorization + header with HTTP requests for log files if the origin of a log file is + the same as the Zuul API. This is useful when build logs are served + through the same authenticated endpoint as the API (e.g. a reverse + proxy). + .. attr:: keystore .. attr:: password diff --git a/tests/unit/test_web.py b/tests/unit/test_web.py index f67a21074c..74643b139d 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -2022,6 +2022,7 @@ class TestInfo(BaseTestWeb): "realms": {}, "default_realm": None, "read_protected": False, + "auth_log_file_requests": False, } }, "stats": { @@ -2082,6 +2083,7 @@ class TestWebCapabilitiesInfo(TestInfo): }, 'default_realm': 'myOIDC1', 'read_protected': False, + 'auth_log_file_requests': False, } if niz is not None: info['info']['capabilities']['auth']['niz'] = niz diff --git a/web/src/actions/build.js b/web/src/actions/build.js index 7aa7f51fd7..b8d5295a30 100644 --- a/web/src/actions/build.js +++ b/web/src/actions/build.js @@ -12,8 +12,6 @@ // License for the specific language governing permissions and limitations // under the License. -import Axios from 'axios' - import * as API from '../api' import { fetchLogfile } from './logfile' @@ -324,7 +322,8 @@ function fetchBuildOutput(buildId, state) { const url = build.log_url.substr(0, build.log_url.lastIndexOf('/') + 1) dispatch(requestBuildOutput()) try { - const response = await Axios.get(url + 'job-output.json.gz') + const auth_log_request = state.info.capabilities?.auth?.auth_log_file_requests === true + const response = await API.getLogFile(url + 'job-output.json.gz', auth_log_request) dispatch(receiveBuildOutput(buildId, response.data)) } catch (error) { if (!error.request) { @@ -335,7 +334,8 @@ function fetchBuildOutput(buildId, state) { } try { // Try without compression - const response = await Axios.get(url + 'job-output.json') + const auth_log_request = state.info.capabilities?.auth?.auth_log_file_requests === true + const response = await API.getLogFile(url + 'job-output.json', auth_log_request) dispatch(receiveBuildOutput(buildId, response.data)) } catch (error) { dispatch(failedBuildOutput(buildId, error, url)) @@ -367,7 +367,8 @@ export function fetchBuildManifest(buildId, state) { artifact.metadata.type === 'zuul_manifest' ) { try { - const response = await Axios.get(artifact.url) + const auth_log_request = state.info.capabilities?.auth?.auth_log_file_requests === true + const response = await API.getLogFile(artifact.url, auth_log_request) return dispatch(receiveBuildManifest(buildId, response.data)) } catch(error) { // Show the error since we expected a manifest but did not diff --git a/web/src/actions/logfile.js b/web/src/actions/logfile.js index 24f986ba37..55c6fc4e47 100644 --- a/web/src/actions/logfile.js +++ b/web/src/actions/logfile.js @@ -12,7 +12,7 @@ // License for the specific language governing permissions and limitations // under the License. -import Axios from 'axios' +import * as API from '../api' export const LOGFILE_FETCH_REQUEST = 'LOGFILE_FETCH_REQUEST' export const LOGFILE_FETCH_SUCCESS = 'LOGFILE_FETCH_SUCCESS' @@ -109,7 +109,8 @@ export function fetchLogfile(buildId, file, state) { const url = build.log_url + file dispatch(requestLogfile()) try { - const response = await Axios.get(url, { transformResponse: [] }) + const auth_log_request = state.info.capabilities?.auth?.auth_log_file_requests === true + const response = await API.getLogFile(url, auth_log_request, { transformResponse: [] }) dispatch(receiveLogfile(buildId, file, response.data)) } catch(error) { dispatch(failedLogfile(error, url)) diff --git a/web/src/api.js b/web/src/api.js index b89f7973a9..42d563d499 100644 --- a/web/src/api.js +++ b/web/src/api.js @@ -127,6 +127,22 @@ function makeRequest(url, method, data) { return res } +function getLogFile(url, authenticate, requestConfig = {}) { + const apiOrigin = new URL(getZuulUrl()).origin + const urlOrigin = new URL(url).origin + let headers = {} + + // If we serve the log files from the same origin as the api, pass auth + // headers to the log endpoint. + // This is optional and can be enabled explicitly by setting the authenticate + // parameter to true + if (authenticate && authToken && urlOrigin === apiOrigin) { + headers['Authorization'] = `Bearer ${authToken}` + } + const config = {headers: headers, ...requestConfig} + return Axios.get(url, config) +} + // Direct APIs function fetchInfo() { return makeRequest('info') @@ -408,6 +424,7 @@ export { fetchUserAuthorizations, getAuthToken, getHomepageUrl, + getLogFile, getStreamUrl, promote, } diff --git a/web/src/api.test.js b/web/src/api.test.js index 209721671c..1ab11bebbf 100644 --- a/web/src/api.test.js +++ b/web/src/api.test.js @@ -1,4 +1,10 @@ -import { getHomepageUrl } from './api' +import { getHomepageUrl, getLogFile, setAuthToken } from './api' +import axios from 'axios' +jest.mock('axios') + +afterEach(() => { + jest.clearAllMocks() +}) it('should should return the homepage url', () => { const homepage = 'https://my-zuul.com/' @@ -61,3 +67,64 @@ it('should return the subdir homepage url', () => { expect(getHomepageUrl()).toEqual(homepage) } }) + +it('should not request logs with auth header per default', () => { + Object.defineProperty(process.env, 'REACT_APP_ZUUL_API', { + value: 'https://example.com/api/' + }) + + // same origin but we're not expecting auth headers, because + // we have not explicitly enabled that + const logFileUrl = 'https://example.com/logs/build-output.txt' + setAuthToken('foobar') + + getLogFile(logFileUrl, false) + expect(axios.get).toHaveBeenCalledTimes(1) + expect(axios.get).toHaveBeenCalledWith(logFileUrl, { headers: {} }) +}) + +it('should request logs with auth header if enabled', () => { + Object.defineProperty(process.env, 'REACT_APP_ZUUL_API', { + value: 'https://example.com/api/' + }) + + const logFileUrl = 'https://example.com/logs/build-output.txt' + setAuthToken('foobar') + + getLogFile(logFileUrl, true) + expect(axios.get).toHaveBeenCalledTimes(1) + expect(axios.get).toHaveBeenCalledWith(logFileUrl, { + headers: { Authorization: 'Bearer foobar' } + }) +}) + +it('should request logs without auth header if origins don\'t match', () => { + Object.defineProperty(process.env, 'REACT_APP_ZUUL_API', { + value: 'https://example.com/api/' + }) + + // api and log endpoint have different origins, so we must not pass auth + // headers + const logFileUrl = 'https://example.org/logs/build-output.txt' + setAuthToken('foobar') + + getLogFile(logFileUrl, true) + expect(axios.get).toHaveBeenCalledTimes(1) + expect(axios.get).toHaveBeenCalledWith(logFileUrl, { headers: {} }) +}) + +it('should pass additional request configs to axios', () => { + Object.defineProperty(process.env, 'REACT_APP_ZUUL_API', { + value: 'https://example.com/api/' + }) + + const logFileUrl = 'https://example.com/logs/build-output.txt' + setAuthToken('foobar') + + getLogFile(logFileUrl, true, { transformResponse: [] }) + expect(axios.get).toHaveBeenCalledTimes(1) + expect(axios.get).toHaveBeenCalledWith(logFileUrl, { + headers: { Authorization: 'Bearer foobar' }, + transformResponse: [], + }) +}) diff --git a/zuul/web/__init__.py b/zuul/web/__init__.py index 7bcf9d37c4..73fefeb63a 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -1500,6 +1500,8 @@ class ZuulWebAPI(object): auth_info['default_realm'] = root_realm read_protected = bool(self.zuulweb.abide.api_root.access_rules) auth_info['read_protected'] = read_protected + auth_info['auth_log_file_requests'] =\ + self.zuulweb.auth_log_file_requests return self._handleInfo(info.toDict()) @cherrypy.expose @@ -1522,6 +1524,8 @@ class ZuulWebAPI(object): auth_info['read_protected'] = read_protected # TODO: remove this after NIZ transition is complete auth_info['niz'] = bool(self.zuulweb.tenant_providers[tenant_name]) + auth_info['auth_log_file_requests'] =\ + self.zuulweb.auth_log_file_requests return self._handleInfo(info) def _handleInfo(self, info): @@ -3038,6 +3042,12 @@ class ZuulWeb(object): self.finger_tls_verify_hostnames = get_default( self.config, 'fingergw', 'tls_verify_hostnames', default=True) + # NOTE: taking this from the zuul.conf for now, but we might want + # to make this configurable per-tenant in the future + self.auth_log_file_requests = get_default( + self.config, "web", "auth_log_file_requests", default=False + ) + api = ZuulWebAPI(self) self.api = api route_map = self.generateRouteMap(