Skip to content

Return empty rather than forbidden on no access #190

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.

Sign up for GitHub

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

Merged
jakub-id merged 5 commits into from
May 5, 2025
Merged

Return empty rather than forbidden on no access #190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6b45df2
Return empty rather than forbidden on no access
jakub-id May 5, 2025
f1398d9
Update broker/api/api-handler.go
jakub-id May 5, 2025
189fe21
Fix suggestion
jakub-id May 5, 2025
86bed1b
Update API spec
jakub-id May 5, 2025
2ee2067
Docs
jakub-id May 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions broker/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ The broker's API uses hyperlinks to connect JSON resources.
If you're using Chrome or another browser to explore the API,
consider using an extension like [JSON Formatter](https://chromewebstore.google.com/detail/json-formatter/bcjindcccaagfpapjjmafapmmgkkhgoa) which allows to easily navigate hyperlinked JSON.

Note that API is also available with base path `/broker` if env
`TENANT_TO_SYMBOL` is defined.
Note that selected read-only API endpoints are accessible with the base path `/broker`
if env `TENANT_TO_SYMBOL` is defined.
This is allows the broker to be used as a FOLIO/Okapi module,
see the [ModuleDescriptor](./descriptors/ModuleDescriptor-template.json) for details.

# Configuration

Expand Down
57 changes: 24 additions & 33 deletions broker/api/api-handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,15 +82,15 @@ func (a *ApiHandler) GetEvents(w http.ResponseWriter, r *http.Request, params oa
Other: logParams,
})
tran, err := a.getIllTranFromParams(ctx, params.RequesterReqId, params.IllTransactionId)
if err != nil && !errors.Is(err, pgx.ErrNoRows) { //DB error
if err != nil { //DB error
if errors.Is(err, pgx.ErrNoRows) {
writeEmpty(w)
return
}
addInternalError(ctx, w, err)
return
}
if !a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) {
addForbiddenError(ctx, w)
return
}
if err != nil && errors.Is(err, pgx.ErrNoRows) {
writeEmpty(w)
return
}
Expand All @@ -116,22 +116,19 @@ func (a *ApiHandler) GetIllTransactions(w http.ResponseWriter, r *http.Request,
Other: map[string]string{"method": "GetIllTransactions"},
})
tran, err := a.getIllTranFromParams(ctx, params.RequesterReqId, nil)
if err != nil && !errors.Is(err, pgx.ErrNoRows) { //DB error
if err != nil { //DB error
if errors.Is(err, pgx.ErrNoRows) {
writeEmpty(w)
return
}
addInternalError(ctx, w, err)
return
}
//TODO filter the list down instead
if !a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) {
addForbiddenError(ctx, w)
return
}
if err != nil && errors.Is(err, pgx.ErrNoRows) {
writeEmpty(w)
return
}
resp := []oapi.IllTransaction{}
if tran.ID != "" {
resp = append(resp, toApiIllTransaction(r, tran))
if a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) {
resp = append(resp, toApiIllTransaction(r, tran))
}
} else {
trans, err := a.illRepo.ListIllTransactions(ctx)
if err != nil { //DB error
Expand All @@ -152,12 +149,16 @@ func (a *ApiHandler) GetIllTransactionsId(w http.ResponseWriter, r *http.Request
Other: map[string]string{"method": "GetIllTransactionsId", "id": id},
})
trans, err := a.illRepo.GetIllTransactionById(ctx, id)
if err != nil && !errors.Is(err, pgx.ErrNoRows) {
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
addNotFoundError(w)
return
}
addInternalError(ctx, w, err)
return
}
if !a.isOwner(&trans, params.XOkapiTenant, params.RequesterSymbol) {
addForbiddenError(ctx, w)
addNotFoundError(w)
return
}
if trans.ID == "" {
Expand Down Expand Up @@ -509,15 +510,15 @@ func (a *ApiHandler) GetLocatedSuppliers(w http.ResponseWriter, r *http.Request,
Other: logParams,
})
tran, err := a.getIllTranFromParams(ctx, params.RequesterReqId, params.IllTransactionId)
if err != nil && !errors.Is(err, pgx.ErrNoRows) {
if err != nil { //DB error
if errors.Is(err, pgx.ErrNoRows) {
writeEmpty(w)
return
}
addInternalError(ctx, w, err)
return
}
if !a.isOwner(&tran, params.XOkapiTenant, params.RequesterSymbol) {
addForbiddenError(ctx, w)
return
}
if err != nil && errors.Is(err, pgx.ErrNoRows) {
writeEmpty(w)
return
}
Expand Down Expand Up @@ -576,16 +577,6 @@ func addInternalError(ctx extctx.ExtendedContext, w http.ResponseWriter, err err
_ = json.NewEncoder(w).Encode(resp)
}

func addForbiddenError(ctx extctx.ExtendedContext, w http.ResponseWriter) {
resp := ErrorMessage{
Error: "forbidden",
}
ctx.Logger().Error("error serving api request", "error", "forbidden")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusForbidden)
_ = json.NewEncoder(w).Encode(resp)
}

func addBadRequestError(ctx extctx.ExtendedContext, w http.ResponseWriter, err error) {
resp := ErrorMessage{
Error: err.Error(),
Expand Down
40 changes: 10 additions & 30 deletions broker/oapi/open-api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ paths:
type: array
items:
$ref: '#/components/schemas/Event'
'400': # Example error response
'400':
description: Bad Request. Invalid query parameters.
content:
application/json:
Expand All @@ -272,17 +272,7 @@ paths:
error:
type: string
description: Error message
'403': # Example error response
description: Forbidden. Invalid tenant.
content:
application/json:
schema:
type: object
properties:
error:
type: string
description: Error message
'500': # Example error response
'500':
description: Internal Server Error
content:
application/json:
Expand Down Expand Up @@ -311,7 +301,7 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/IllTransaction'
'400': # Example error response
'400':
description: Bad Request. Invalid query parameters.
content:
application/json:
Expand All @@ -321,8 +311,8 @@ paths:
error:
type: string
description: Error message
'403': # Example error response
description: Forbidden. Invalid tenant.
'404':
description: Not Found. ILL transaction not found.
content:
application/json:
schema:
Expand All @@ -331,7 +321,7 @@ paths:
error:
type: string
description: Error message
'500': # Example error response
'500':
description: Internal Server Error
content:
application/json:
Expand Down Expand Up @@ -373,7 +363,7 @@ paths:
type: array
items:
$ref: '#/components/schemas/IllTransaction'
'400': # Example error response
'400':
description: Bad Request. Invalid query parameters.
content:
application/json:
Expand All @@ -383,7 +373,7 @@ paths:
error:
type: string
description: Error message
'403': # Example error response
'403':
description: Forbidden. Invalid tenant.
content:
application/json:
Expand All @@ -393,7 +383,7 @@ paths:
error:
type: string
description: Error message
'500': # Example error response
'500':
description: Internal Server Error
content:
application/json:
Expand Down Expand Up @@ -525,16 +515,6 @@ paths:
error:
type: string
description: Error message
'403': # Example error response
description: Forbidden. Invalid tenant.
content:
application/json:
schema:
type: object
properties:
error:
type: string
description: Error message
'500':
description: Internal Server Error
content:
Expand All @@ -544,4 +524,4 @@ paths:
properties:
error:
type: string
description: Error message
description: Error message
31 changes: 20 additions & 11 deletions broker/test/api/api-handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,20 +238,19 @@ func TestBrokerCRUD(t *testing.T) {
assert.Equal(t, getLocalhostWithPort()+"/broker/events?ill_transaction_id="+url.PathEscape(illId), tran.EventsLink)
assert.Equal(t, getLocalhostWithPort()+"/broker/located_suppliers?ill_transaction_id="+url.PathEscape(illId), tran.LocatedSuppliersLink)

httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "ruc", http.StatusForbidden)

httpGet(t, "/broker/ill_transactions/"+illId, "ruc", http.StatusForbidden)

httpGet(t, "/broker/ill_transactions/"+illId, "", http.StatusForbidden)
httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "diku", http.StatusOK)
httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "ruc", http.StatusNotFound)
httpGet(t, "/broker/ill_transactions/"+illId, "ruc", http.StatusNotFound)
httpGet(t, "/broker/ill_transactions/"+illId, "", http.StatusNotFound)

body = httpGet(t, "/broker/ill_transactions/"+illId+"?requester_symbol="+url.QueryEscape("ISIL:DK-DIKU"), "", http.StatusOK)
err = json.Unmarshal(body, &tran)
assert.NoError(t, err)
assert.Equal(t, illId, tran.ID)

httpGet(t, "/broker/ill_transactions", "diku", http.StatusForbidden)
assert.Equal(t, 1, len(httpGetTrans(t, "/broker/ill_transactions", "diku", http.StatusOK)))

httpGet(t, "/broker/ill_transactions", "ruc", http.StatusForbidden)
assert.Equal(t, 0, len(httpGetTrans(t, "/broker/ill_transactions", "ruc", http.StatusOK)))

body = httpGet(t, "/broker/ill_transactions?requester_req_id="+url.QueryEscape(reqReqId), "diku", http.StatusOK)
var trans []oapi.IllTransaction
Expand All @@ -276,9 +275,9 @@ func TestBrokerCRUD(t *testing.T) {
assert.Len(t, supps, 1)
assert.Equal(t, locSup.ID, supps[0].ID)

httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusForbidden)
assert.Equal(t, []byte("[]"), httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusOK))

httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusForbidden)
assert.Equal(t, []byte("[]"), httpGet(t, "/broker/located_suppliers?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusOK))

eventId := test.GetEventId(t, eventRepo, illId, events.EventTypeNotice, events.EventStatusSuccess, events.EventNameMessageRequester)

Expand All @@ -301,9 +300,9 @@ func TestBrokerCRUD(t *testing.T) {
assert.Len(t, events, 1)
assert.Equal(t, eventId, events[0].ID)

httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusForbidden)
assert.Equal(t, []byte("[]"), httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(reqReqId), "ruc", http.StatusOK))

httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusForbidden)
assert.Equal(t, []byte("[]"), httpGet(t, "/broker/events?requester_req_id="+url.QueryEscape(uuid.NewString()), "diku", http.StatusOK))
}

func TestPeersCRUD(t *testing.T) {
Expand Down Expand Up @@ -561,6 +560,16 @@ func httpRequest(t *testing.T, method string, uriPath string, reqbytes []byte, t
return body
}

func httpGetTrans(t *testing.T, uriPath string, tenant string, expectStatus int) []oapi.IllTransaction {
body := httpRequest(t, "GET", uriPath, nil, tenant, expectStatus)
var res []oapi.IllTransaction
err := json.Unmarshal(body, &res)
if err != nil {
t.Errorf("Failed to unmarshal json: %s", err)
}
return res
}

func httpGet(t *testing.T, uriPath string, tenant string, expectStatus int) []byte {
return httpRequest(t, "GET", uriPath, nil, tenant, expectStatus)
}
Expand Down
Loading