fix: authentication flow should abort early

[fmartingr]

• Ensures that the connection is aborted from the Auth middleware when the authentication is unsuccessful.
• Modified the main middleware test to ensure that we are not filtering out extra response data
• Added helper methods to testutil.TestResponse
• Fixed some typos
• Fixed tests for the tags and bookmarks api v1

[Monirzadeh]

Hi
if you send an unauthorized request it will not reach to the any endpoint in internal/http/routes/api/v1/bookmarks.go
Why is it important?
in #885 for public bookmarks you can't accesses readable content bemuse your request not reach to the endpoint (if be unauthorized)
you can test that in other way
send a update cache request to the

http:https://127.0.0.1:8080/api/v1/bookmarks/cache

if you are unauthorized than you get 401 but this status code not came from this line

shiori/internal/http/routes/api/v1/bookmarks.go

Lines 70 to 73 in c77a542

	if !ctx.UserIsLogged() {
	response.SendError(c, http.StatusForbidden, nil)
	return
	}

it force us to all api in internal/http/routes/api/v1/bookmarks.go be authorize (and public status be pointless on endpoint in there)
dose you want this?

[fmartingr]

Hi if you send an unauthorized request it will not reach to the any endpoint in internal/http/routes/api/v1/bookmarks.go Why is it important? in #885 for public bookmarks you can't accesses readable content bemuse your request not reach to the endpoint (if be unauthorized) you can test that in other way send a update cache request to the

http:https://127.0.0.1:8080/api/v1/bookmarks/cache

if you are unauthorized than you get 401 but this status code not came from this line

shiori/internal/http/routes/api/v1/bookmarks.go

Lines 70 to 73 in c77a542

	if !ctx.UserIsLogged() {
	response.SendError(c, http.StatusForbidden, nil)
	return
	}

it force us to all api in internal/http/routes/api/v1/bookmarks.go be authorize (and public status be pointless on endpoint in there) dose you want this?

Right now all handlers under the bookmark endpoint require authentication, so that's good as it is. When we add new mixed handlers we could add the middleware just above the endpoints that require it. Is this something you need for your PR?

[fmartingr]

@Monirzadeh Check dff17e1

[fmartingr]

@Monirzadeh Check dff17e1

Ugh, now I remember why I did it the other way, routes were easier to test.

[Monirzadeh]

Hi if you send an unauthorized request it will not reach to the any endpoint in internal/http/routes/api/v1/bookmarks.go Why is it important? in #885 for public bookmarks you can't accesses readable content bemuse your request not reach to the endpoint (if be unauthorized) you can test that in other way send a update cache request to the

http:https://127.0.0.1:8080/api/v1/bookmarks/cache

if you are unauthorized than you get 401 but this status code not came from this line

shiori/internal/http/routes/api/v1/bookmarks.go

Lines 70 to 73 in c77a542

	if !ctx.UserIsLogged() {
	response.SendError(c, http.StatusForbidden, nil)
	return
	}

it force us to all api in internal/http/routes/api/v1/bookmarks.go be authorize (and public status be pointless on endpoint in there) dose you want this?

Right now all handlers under the bookmark endpoint require authentication, so that's good as it is. When we add new mixed handlers we could add the middleware just above the endpoints that require it. Is this something you need for your PR?

so for example if i need user can acsses readable content of public bookmark (in unauthorized mode) i can't put that in internal/http/routes/api/v1/bookmarks.go
what should i do in this specific situation?

thinking about an app on phone that user can browse public bookmark from multiple shiori server.
so in this situation you need get unauthorized request in endpoint but filter them based on public status in endpoint.

[Monirzadeh]

in long term i am thinking about something that user can share there bookmarks with each other (if they want and be disable by default). it is not a mature idea right now.
something like pocket discovery but it work like federation program (mastadon, Pixelfed, etc) based on ActivityPub.

[Monirzadeh]

it is fine to me right now.
we can spend time in that specific situation if it necessary later.
just please fix that failed unit tests.
thanks for this fix 👍

[Monirzadeh]

so with this new change we should remove this line too

shiori/internal/http/routes/api/v1/bookmarks.go

Lines 70 to 73 in c77a542

	if !ctx.UserIsLogged() {
	response.SendError(c, http.StatusForbidden, nil)
	return
	}

[Monirzadeh]

thanks 👍