feat: auth service-to-service api #3148
Conversation
github-actions
bot
added
type: tests
| private async request<T = any>( | ||
| path: string, | ||
| options: RequestInit | ||
| ): Promise<T> { | ||
| const response = await fetch(`${this.baseUrl}${path}`, options) | ||
|
|
||
| if (!response.ok) { | ||
| let errorDetails | ||
| try { | ||
| errorDetails = await response.json() | ||
| } catch { | ||
| errorDetails = { message: response.statusText } | ||
| } | ||
|
|
||
| throw new AuthServiceClientError( | ||
| `Auth Service Client Error: ${response.status} ${response.statusText}`, | ||
| response.status, | ||
| errorDetails | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| response.status === 204 || | ||
| response.headers.get('Content-Length') === '0' | ||
| ) { | ||
| return undefined as T | ||
| } | ||
|
|
||
| const contentType = response.headers.get('Content-Type') | ||
| if (contentType && contentType.includes('application/json')) { | ||
| try { | ||
| return (await response.json()) as T | ||
| } catch (error) { | ||
| throw new AuthServiceClientError( | ||
| `Failed to parse JSON response from ${path}`, | ||
| response.status | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return (await response.text()) as T | ||
| } |
There was a problem hiding this comment.
I opted to use the native fetch here although there were some behaviors I wasnt quite sure about. I tested with axios and mirrored how that works:
- not OK response (400, 500) etc. throws
- 204 returns
undefined - Success without body (200, 201, etc.) returns the response.text (just
''if not set) - Interface accepts a generic which asserts the return type, although technically it could still return a
stringorundefinedin the cases described above.
| async function createTenant( | ||
| deps: ServiceDependencies, | ||
| ctx: CreateContext | ||
| ): Promise<void> { | ||
| const { body } = ctx.request | ||
|
|
||
| await deps.tenantService.create(body) | ||
|
|
||
| ctx.status = 204 | ||
| } |
There was a problem hiding this comment.
@mkurapov influenced by the pattern we've been talking about in the POC meetings. Not 201 with the resource returned. Originally I went with 201 but that really should have a body: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/201
thus the 204 No Content for this and the update and delete.
| function toTenantResponse(tenant: Tenant): TenantResponse { | ||
| return { | ||
| id: tenant.id, | ||
| idpConsentUrl: tenant.idpConsentUrl, | ||
| idpSecret: tenant.idpSecret | ||
| } | ||
| } |
There was a problem hiding this comment.
to filter out the created, deleted, update timestamps basically. Figured we'd keep those in the return from the service layer and filter out here. Although I considered not returning them from the service since in theory maybe those are purely business logic concerns? Kinda academic at this point though.
| public tenant = { | ||
| get: (id: string) => | ||
| this.request<Tenant>(`/tenant/${id}`, { method: 'GET' }), | ||
| create: (data: Omit<Tenant, 'id'>) => | ||
| this.request('/tenant', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(data) | ||
| }), | ||
| update: (id: string, data: Partial<Omit<Tenant, 'id'>>) => | ||
| this.request(`/tenant/${id}`, { | ||
| method: 'PATCH', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(data) | ||
| }), | ||
| delete: (id: string) => this.request(`/tenant/${id}`, { method: 'DELETE' }) | ||
| } |
There was a problem hiding this comment.
could probably move these methods and the Tenant interface out of this file if we want but opted to just put it here since it's really the only thing we need for the forseeable future.
| } | ||
| } | ||
|
|
||
| export class AuthServiceClient { |
There was a problem hiding this comment.
Almost feels like this should be a different package outside of backend but this is the only consumer at this point so I felt like this keeps it simpler.
There was a problem hiding this comment.
Intention is to add this as a dep to the tenant service, when thats merged in.
Changes proposed in this pull request
ServiceAPIServertoauthServiceAPIServerauthServiceAPIClienttobackendthat should be used bybackendtenant service. The intention is to update that here when that PR gets merged in: feat(backend): tenants service (#3123) #3140Context
fixes: #3125
Checklist
fixes #numberuser-docslabel (if necessary)