diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst index 141bdc5750..937504a54e 100644 --- a/doc/source/configuration.rst +++ b/doc/source/configuration.rst @@ -997,6 +997,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 3cf42808bd..7d2524b9bd 100644 --- a/tests/unit/test_web.py +++ b/tests/unit/test_web.py @@ -2132,6 +2132,7 @@ class TestInfo(BaseTestWeb): "realms": {}, "default_realm": None, "read_protected": False, + "auth_log_file_requests": False, } }, "stats": { @@ -2192,6 +2193,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 a7ed850b04..d390c297d9 100755 --- a/zuul/web/__init__.py +++ b/zuul/web/__init__.py @@ -1501,6 +1501,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 @@ -1523,6 +1525,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): @@ -3048,6 +3052,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(