Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this line be moved into the open routes group of handlers above, rather than creating a new group for it?

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" {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the unauthorised200 parameter necessary? It looks like the frontend code will already accept a 200 response with null as the JSON response.

Please correct me if I'm wrong, I'm new here, but I think if null can be returned here, it looks like no frontend changes would be needed.

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
.get<User | null>('/user', { unauthorised200: String(unauthorised200) })
.then((response) => {
if (unauthorised200 && response && 'error' in response) {
return null;
}
return response;
})
.catch(() => null);
}

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