Skip to content

Commit

Permalink
add unauthorised200 parameter to /user endpoint
Browse files Browse the repository at this point in the history
  • Loading branch information
paskal committed May 2, 2024
1 parent 877765c commit 52a9ab7
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 7 deletions.
9 changes: 8 additions & 1 deletion backend/app/rest/api/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,17 @@ func (s *Rest) routes() chi.Router {
rauth.Use(middleware.Timeout(30 * time.Second))
rauth.Use(tollbooth_chi.LimitHandler(tollbooth.NewLimiter(10, nil)))
rauth.Use(authMiddleware.Auth, matchSiteID, middleware.NoCache, logInfoWithBody)
rauth.Get("/user", s.privRest.userInfoCtrl)
rauth.Get("/userdata", s.privRest.userAllDataCtrl)
})

// protected routes, user is set but checked inside the handlers
rapi.Group(func(rauthOptional chi.Router) {
rauthOptional.Use(middleware.Timeout(30 * time.Second))
rauthOptional.Use(tollbooth_chi.LimitHandler(tollbooth.NewLimiter(10, nil)))
rauthOptional.Use(authMiddleware.Trace, middleware.NoCache, logInfoWithBody)
rauthOptional.Get("/user", s.privRest.userInfoCtrl)
})

// admin routes, require auth and admin users only
rapi.Route("/admin", func(radmin chi.Router) {
radmin.Use(middleware.Timeout(30 * time.Second))
Expand Down
16 changes: 14 additions & 2 deletions backend/app/rest/api/rest_private.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,9 +236,21 @@ func (s *private) updateCommentCtrl(w http.ResponseWriter, r *http.Request) {
render.JSON(w, r, res)
}

// GET /user?site=siteID - returns user info
// GET /user?site=siteID&unauthorised200=false - returns user info, with unauthorised200=true returns 200 with error message
func (s *private) userInfoCtrl(w http.ResponseWriter, r *http.Request) {
user := rest.MustGetUserInfo(r)
user, err := rest.GetUserInfo(r)
if err != nil {
if r.URL.Query().Get("unauthorised200") == "true" {
render.JSON(w, r, R.JSON{"error": err.Error()})
return
}
http.Error(w, "Unauthorized", http.StatusUnauthorized)
return
}

// as user is set, call matchSiteID middleware to verify SiteID match
matchSiteID(http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {})).ServeHTTP(w, r)

if siteID := r.URL.Query().Get("site"); siteID != "" {
user.Verified = s.dataService.IsVerified(siteID, user.ID)

Expand Down
38 changes: 38 additions & 0 deletions backend/app/rest/api/rest_private_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1391,6 +1391,44 @@ func TestRest_TelegramNotification(t *testing.T) {
assert.Empty(t, mockDestination.Get()[3].Telegrams)
}

func TestRest_UserUnauthorised200(t *testing.T) {
ts, _, teardown := startupT(t)
defer teardown()

client := &http.Client{Timeout: 1 * time.Second}
defer client.CloseIdleConnections()
req, err := http.NewRequest("GET", ts.URL+"/api/v1/user?site=remark42", http.NoBody)
require.NoError(t, err)
resp, err := client.Do(req)
require.NoError(t, err)
require.Equal(t, http.StatusUnauthorized, resp.StatusCode)
body, err := io.ReadAll(resp.Body)
assert.NoError(t, resp.Body.Close())
assert.NoError(t, err)
assert.Equal(t, "Unauthorized\n", string(body))

req, err = http.NewRequest("GET", ts.URL+"/api/v1/user?site=remark42&unauthorised200=true", http.NoBody)
require.NoError(t, err)
resp, err = client.Do(req)
require.NoError(t, err)
require.Equal(t, http.StatusOK, resp.StatusCode, "should fail but with status code 200 due to the unauthorised200 param set")
body, err = io.ReadAll(resp.Body)
assert.NoError(t, resp.Body.Close())
assert.NoError(t, err)
assert.Equal(t, `{"error":"can't extract user info from the token: user can't be parsed"}`+"\n", string(body))

req, err = http.NewRequest("GET", ts.URL+"/api/v1/user?site=wrong_site&unauthorised200=true", http.NoBody)
require.NoError(t, err)
req.Header.Add("X-JWT", devToken)
resp, err = client.Do(req)
require.NoError(t, err)
require.Equal(t, http.StatusForbidden, resp.StatusCode, "should fail due to site mismatch")
body, err = io.ReadAll(resp.Body)
assert.NoError(t, resp.Body.Close())
assert.NoError(t, err)
assert.Contains(t, string(body), "Access denied\n")
}

func TestRest_UserAllData(t *testing.T) {
ts, srv, teardown := startupT(t)
defer teardown()
Expand Down
12 changes: 10 additions & 2 deletions frontend/apps/remark42/app/common/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,16 @@ export const removeMyComment = (id: Comment['id']): Promise<void> =>

export const getPreview = (text: string): Promise<string> => apiFetcher.post('/preview', {}, { text });

export function getUser(): Promise<User | null> {
return apiFetcher.get<User | null>('/user').catch(() => null);
export function getUser(unauthorised200= true): Promise<User | null> {
return apiFetcher

Check warning on line 64 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L64

Added line #L64 was not covered by tests
.get<User | null>('/user', { unauthorised200: String(unauthorised200) })
.then((response) => {

Check warning on line 66 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L66

Added line #L66 was not covered by tests
if (unauthorised200 && response && 'error' in response) {
return null;

Check warning on line 68 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L68

Added line #L68 was not covered by tests
}
return response;

Check warning on line 70 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L70

Added line #L70 was not covered by tests
})
.catch(() => null);

Check warning on line 72 in frontend/apps/remark42/app/common/api.ts

View check run for this annotation

Codecov / codecov/patch

frontend/apps/remark42/app/common/api.ts#L72

Added line #L72 was not covered by tests
}

export const uploadImage = (image: File): Promise<Image> => {
Expand Down
12 changes: 10 additions & 2 deletions frontend/packages/api/clients/public.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,16 @@ export function createPublicClient({ siteId: site, baseUrl }: ClientParams) {
/**
* Get current authorized user
*/
async function getUser(): Promise<User | null> {
return fetcher.get<User | null>('/user').catch(() => null)
async function getUser(unauthorised200= true): Promise<User | null> {
return fetcher
.get<User | null>('/user', { unauthorised200: String(unauthorised200) })
.then((response) => {
if (unauthorised200 && response && 'error' in response) {
return null;
}
return response;
})
.catch(() => null);
}

/**
Expand Down

0 comments on commit 52a9ab7

Please sign in to comment.