Skip to content

Commit 636aa9c

Browse files
authored
Merge pull request #438 from IQSS/437-access-repository-omit-credentials
AccessRepository.ts omits credentials if using Bearer Token Authorization
2 parents 9562e61 + a4458ff commit 636aa9c

3 files changed

Lines changed: 241 additions & 33 deletions

File tree

Lines changed: 117 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1+
import { ApiConfig, DataverseApiAuthMechanism } from '../../../core/infra/repositories/ApiConfig'
2+
import { WriteError } from '../../../core/domain/repositories/WriteError'
3+
import { ApiConstants } from '../../../core/infra/repositories/ApiConstants'
14
import { ApiRepository } from '../../../core/infra/repositories/ApiRepository'
5+
import {
6+
buildRequestConfig,
7+
buildRequestUrl
8+
} from '../../../core/infra/repositories/apiConfigBuilders'
29
import { GuestbookResponseDTO } from '../../domain/dtos/GuestbookResponseDTO'
310
import { IAccessRepository } from '../../domain/repositories/IAccessRepository'
411

@@ -13,14 +20,7 @@ export class AccessRepository extends ApiRepository implements IAccessRepository
1320
const endpoint = this.buildApiEndpoint(`${this.accessResourceName}/datafile`, undefined, fileId)
1421
const queryParams = format ? { signed: true, format } : { signed: true }
1522

16-
return this.doPost(endpoint, guestbookResponse, queryParams)
17-
.then((response) => {
18-
const signedUrl = response.data.data.signedUrl
19-
return signedUrl
20-
})
21-
.catch((error) => {
22-
throw error
23-
})
23+
return await this.submitGuestbookDownload(endpoint, guestbookResponse, queryParams)
2424
}
2525

2626
public async submitGuestbookForDatafilesDownload(
@@ -30,21 +30,14 @@ export class AccessRepository extends ApiRepository implements IAccessRepository
3030
): Promise<string> {
3131
const queryParams = format ? { signed: true, format } : { signed: true }
3232

33-
return this.doPost(
33+
return await this.submitGuestbookDownload(
3434
this.buildApiEndpoint(
3535
this.accessResourceName,
3636
`datafiles/${Array.isArray(fileIds) ? fileIds.join(',') : fileIds}`
3737
),
3838
guestbookResponse,
3939
queryParams
4040
)
41-
.then((response) => {
42-
const signedUrl = response.data.data.signedUrl
43-
return signedUrl
44-
})
45-
.catch((error) => {
46-
throw error
47-
})
4841
}
4942

5043
public async submitGuestbookForDatasetDownload(
@@ -59,14 +52,7 @@ export class AccessRepository extends ApiRepository implements IAccessRepository
5952
)
6053
const queryParams = format ? { signed: true, format } : { signed: true }
6154

62-
return this.doPost(endpoint, guestbookResponse, queryParams)
63-
.then((response) => {
64-
const signedUrl = response.data.data.signedUrl
65-
return signedUrl
66-
})
67-
.catch((error) => {
68-
throw error
69-
})
55+
return await this.submitGuestbookDownload(endpoint, guestbookResponse, queryParams)
7056
}
7157

7258
public async submitGuestbookForDatasetVersionDownload(
@@ -82,13 +68,112 @@ export class AccessRepository extends ApiRepository implements IAccessRepository
8268
)
8369
const queryParams = format ? { signed: true, format } : { signed: true }
8470

85-
return this.doPost(endpoint, guestbookResponse, queryParams)
86-
.then((response) => {
87-
const signedUrl = response.data.data.signedUrl
88-
return signedUrl
89-
})
90-
.catch((error) => {
91-
throw error
92-
})
71+
return await this.submitGuestbookDownload(endpoint, guestbookResponse, queryParams)
72+
}
73+
74+
private async submitGuestbookDownload(
75+
apiEndpoint: string,
76+
guestbookResponse: GuestbookResponseDTO,
77+
queryParams: object
78+
): Promise<string> {
79+
const requestConfig = buildRequestConfig(
80+
true,
81+
queryParams,
82+
ApiConstants.CONTENT_TYPE_APPLICATION_JSON
83+
)
84+
const response = await fetch(
85+
this.buildUrlWithQueryParams(buildRequestUrl(apiEndpoint), queryParams),
86+
{
87+
method: 'POST',
88+
headers: this.buildFetchHeaders(requestConfig.headers),
89+
credentials: this.getFetchCredentials(requestConfig.withCredentials),
90+
body: JSON.stringify(guestbookResponse)
91+
}
92+
).catch((error) => {
93+
throw new WriteError(error instanceof Error ? error.message : String(error))
94+
})
95+
96+
const responseData = await this.parseResponseBody(response)
97+
98+
if (!response.ok) {
99+
throw new WriteError(this.buildFetchErrorMessage(response.status, responseData))
100+
}
101+
102+
return this.getSignedUrlOrThrow(responseData)
103+
}
104+
105+
private getFetchCredentials(withCredentials?: boolean): RequestCredentials | undefined {
106+
if (ApiConfig.dataverseApiAuthMechanism === DataverseApiAuthMechanism.BEARER_TOKEN) {
107+
return 'omit'
108+
}
109+
110+
if (withCredentials) {
111+
return 'include'
112+
}
113+
114+
return undefined
115+
}
116+
117+
private buildUrlWithQueryParams(requestUrl: string, queryParams: object): string {
118+
const url = new URL(requestUrl)
119+
120+
Object.entries(queryParams).forEach(([key, value]) => {
121+
if (value !== undefined && value !== null) {
122+
url.searchParams.append(key, String(value))
123+
}
124+
})
125+
126+
return url.toString()
127+
}
128+
129+
private buildFetchHeaders(headers?: Record<string, unknown>): Record<string, string> {
130+
const fetchHeaders: Record<string, string> = {}
131+
132+
if (!headers) {
133+
return fetchHeaders
134+
}
135+
136+
Object.entries(headers).forEach(([key, value]) => {
137+
if (value !== undefined) {
138+
fetchHeaders[key] = String(value)
139+
}
140+
})
141+
142+
return fetchHeaders
143+
}
144+
145+
private async parseResponseBody(response: Response): Promise<any> {
146+
const contentType = response.headers.get('content-type') ?? ''
147+
148+
if (contentType.includes('application/json')) {
149+
return await response.json()
150+
}
151+
152+
const responseText = await response.text()
153+
154+
try {
155+
return JSON.parse(responseText)
156+
} catch {
157+
return responseText
158+
}
159+
}
160+
161+
private buildFetchErrorMessage(status: number, responseData: any): string {
162+
const message =
163+
typeof responseData === 'string'
164+
? responseData
165+
: responseData?.message || responseData?.data?.message || 'unknown error'
166+
167+
return `[${status}] ${message}`
168+
}
169+
170+
private getSignedUrlOrThrow(responseData: any): string {
171+
const signedUrl = responseData?.data?.signedUrl
172+
173+
if (typeof signedUrl !== 'string' || signedUrl.length === 0) {
174+
throw new WriteError('Missing signedUrl in access download response.')
175+
}
176+
177+
return signedUrl
93178
}
94179
}

test/integration/access/AccessRepository.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,14 @@ describe('AccessRepository', () => {
6363
})
6464

6565
describe('submitGuestbookForDatafileDownload', () => {
66-
test('should return signed url for datafile download', async () => {
66+
test('should return signed url that can be used to download the datafile', async () => {
6767
const actual = await sut.submitGuestbookForDatafileDownload(testFileId, guestbookResponse)
68+
const downloadResponse = await fetch(actual)
6869

6970
expect(actual).toEqual(expect.any(String))
7071
expect(() => new URL(actual)).not.toThrow()
72+
expect(downloadResponse.ok).toBe(true)
73+
await expect(downloadResponse.text()).resolves.toBe('test file 1\n')
7174
})
7275

7376
test('should return error when datafile does not exist', async () => {
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
/**
2+
* @jest-environment jsdom
3+
*/
4+
5+
import { AccessRepository } from '../../../src/access/infra/repositories/AccessRepository'
6+
import { GuestbookResponseDTO } from '../../../src/access/domain/dtos/GuestbookResponseDTO'
7+
import { WriteError } from '../../../src/core/domain/repositories/WriteError'
8+
import {
9+
ApiConfig,
10+
DataverseApiAuthMechanism
11+
} from '../../../src/core/infra/repositories/ApiConfig'
12+
import { TestConstants } from '../../testHelpers/TestConstants'
13+
14+
describe('AccessRepository', () => {
15+
const sut = new AccessRepository()
16+
const originalFetch = global.fetch
17+
const guestbookResponse: GuestbookResponseDTO = {
18+
guestbookResponse: {
19+
answers: [{ id: 1, value: 'question 1' }]
20+
}
21+
}
22+
23+
beforeEach(() => {
24+
window.localStorage.setItem(
25+
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY,
26+
JSON.stringify(TestConstants.TEST_DUMMY_BEARER_TOKEN)
27+
)
28+
})
29+
30+
afterEach(() => {
31+
global.fetch = originalFetch
32+
window.localStorage.clear()
33+
})
34+
35+
test('uses fetch with credentials omit for bearer token auth', async () => {
36+
ApiConfig.init(
37+
TestConstants.TEST_API_URL,
38+
DataverseApiAuthMechanism.BEARER_TOKEN,
39+
undefined,
40+
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY
41+
)
42+
43+
const fetchMock = jest.fn().mockResolvedValue({
44+
ok: true,
45+
status: 200,
46+
headers: new Headers({ 'content-type': 'application/json' }),
47+
json: jest.fn().mockResolvedValue({
48+
data: {
49+
signedUrl: 'https://signed.dataset'
50+
}
51+
})
52+
} as unknown as Response)
53+
54+
global.fetch = fetchMock as typeof fetch
55+
56+
const actual = await sut.submitGuestbookForDatasetDownload(123, guestbookResponse, 'original')
57+
58+
expect(fetchMock).toHaveBeenCalledWith(
59+
`${TestConstants.TEST_API_URL}/access/dataset/123?signed=true&format=original`,
60+
{
61+
method: 'POST',
62+
headers: {
63+
'Content-Type': 'application/json',
64+
Authorization: `Bearer ${TestConstants.TEST_DUMMY_BEARER_TOKEN}`
65+
},
66+
credentials: 'omit',
67+
body: JSON.stringify(guestbookResponse)
68+
}
69+
)
70+
expect(actual).toBe('https://signed.dataset')
71+
})
72+
73+
test('parses signed url from a JSON body when content-type is incorrect', async () => {
74+
ApiConfig.init(
75+
TestConstants.TEST_API_URL,
76+
DataverseApiAuthMechanism.BEARER_TOKEN,
77+
undefined,
78+
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY
79+
)
80+
81+
const fetchMock = jest.fn().mockResolvedValue({
82+
ok: true,
83+
status: 200,
84+
headers: new Headers({ 'content-type': 'text/plain' }),
85+
text: jest
86+
.fn()
87+
.mockResolvedValue(JSON.stringify({ data: { signedUrl: 'https://signed.text' } }))
88+
} as unknown as Response)
89+
90+
global.fetch = fetchMock as typeof fetch
91+
92+
await expect(sut.submitGuestbookForDatasetDownload(123, guestbookResponse)).resolves.toBe(
93+
'https://signed.text'
94+
)
95+
})
96+
97+
test('throws WriteError when signedUrl is missing from a successful response', async () => {
98+
ApiConfig.init(
99+
TestConstants.TEST_API_URL,
100+
DataverseApiAuthMechanism.BEARER_TOKEN,
101+
undefined,
102+
TestConstants.TEST_BEARER_TOKEN_LOCAL_STORAGE_KEY
103+
)
104+
105+
const fetchMock = jest.fn().mockResolvedValue({
106+
ok: true,
107+
status: 200,
108+
headers: new Headers({ 'content-type': 'application/json' }),
109+
json: jest.fn().mockResolvedValue({
110+
data: {}
111+
})
112+
} as unknown as Response)
113+
114+
global.fetch = fetchMock as typeof fetch
115+
116+
await expect(sut.submitGuestbookForDatasetDownload(123, guestbookResponse)).rejects.toThrow(
117+
new WriteError('Missing signedUrl in access download response.')
118+
)
119+
})
120+
})

0 commit comments

Comments
 (0)