Skip to content

C
canifa_note
Unverified Commit 4ed9a3a0 authored by boojack's avatar boojack Committed by GitHub
Browse files
Options

refactor: migrate auth routes to v1 package (#1841) 

* feat: add api v1 packages

* chore: migrate auth to v1

* chore: update test
parent f1d85eea
Hide whitespace changes
Inline Side-by-side
Showing with 180 additions and 131 deletions
api/auth.go deleted 100644 → 0
View file @ f1d85eea
package api
type SignIn struct {
Username string `json:"username"`
Password string `json:"password"`
}
type SSOSignIn struct {
IdentityProviderID int `json:"identityProviderId"`
Code string `json:"code"`
RedirectURI string `json:"redirectUri"`
}
type SignUp struct {
Username string `json:"username"`
Password string `json:"password"`
}
server/auth.go → api/v1/auth.go
View file @ 4ed9a3a0
package server
package v1
import (
"encoding/json"
......@@ -6,21 +6,37 @@ import (
"net/http"
"regexp"
"github.com/labstack/echo/v4"
"github.com/pkg/errors"
"github.com/usememos/memos/api"
"github.com/usememos/memos/common"
"github.com/usememos/memos/plugin/idp"
"github.com/usememos/memos/plugin/idp/oauth2"
"github.com/usememos/memos/server/auth"
"github.com/usememos/memos/store"
"github.com/labstack/echo/v4"
"golang.org/x/crypto/bcrypt"
)
func (s *Server) registerAuthRoutes(g *echo.Group, secret string) {
type SignIn struct {
Username string `json:"username"`
Password string `json:"password"`
}
type SSOSignIn struct {
IdentityProviderID int `json:"identityProviderId"`
Code string `json:"code"`
RedirectURI string `json:"redirectUri"`
}
type SignUp struct {
Username string `json:"username"`
Password string `json:"password"`
}
func (s *APIV1Service) registerAuthRoutes(g *echo.Group, secret string) {
g.POST("/auth/signin", func(c echo.Context) error {
ctx := c.Request().Context()
signin := &api.SignIn{}
signin := &SignIn{}
if err := json.NewDecoder(c.Request().Body).Decode(signin); err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "Malformatted signin request").SetInternal(err)
}
......@@ -44,18 +60,18 @@ func (s *Server) registerAuthRoutes(g *echo.Group, secret string) {
return echo.NewHTTPError(http.StatusUnauthorized, "Incorrect login credentials, please try again")
}
if err := GenerateTokensAndSetCookies(c, user, secret); err != nil {
if err := auth.GenerateTokensAndSetCookies(c, user, secret); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to generate tokens").SetInternal(err)
}
if err := s.createUserAuthSignInActivity(c, user); err != nil {
if err := s.createAuthSignInActivity(c, user); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to create activity").SetInternal(err)
}
return c.JSON(http.StatusOK, composeResponse(user))
return c.JSON(http.StatusOK, user)
})
g.POST("/auth/signin/sso", func(c echo.Context) error {
ctx := c.Request().Context()
signin := &api.SSOSignIn{}
signin := &SSOSignIn{}
if err := json.NewDecoder(c.Request().Body).Decode(signin); err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "Malformatted signin request").SetInternal(err)
}
......@@ -128,18 +144,18 @@ func (s *Server) registerAuthRoutes(g *echo.Group, secret string) {
return echo.NewHTTPError(http.StatusForbidden, fmt.Sprintf("User has been archived with username %s", userInfo.Identifier))
}
if err := GenerateTokensAndSetCookies(c, user, secret); err != nil {
if err := auth.GenerateTokensAndSetCookies(c, user, secret); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to generate tokens").SetInternal(err)
}
if err := s.createUserAuthSignInActivity(c, user); err != nil {
if err := s.createAuthSignInActivity(c, user); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to create activity").SetInternal(err)
}
return c.JSON(http.StatusOK, composeResponse(user))
return c.JSON(http.StatusOK, user)
})
g.POST("/auth/signup", func(c echo.Context) error {
ctx := c.Request().Context()
signup := &api.SignUp{}
signup := &SignUp{}
if err := json.NewDecoder(c.Request().Body).Decode(signup); err != nil {
return echo.NewHTTPError(http.StatusBadRequest, "Malformatted signup request").SetInternal(err)
}
......@@ -196,23 +212,23 @@ func (s *Server) registerAuthRoutes(g *echo.Group, secret string) {
if err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to create user").SetInternal(err)
}
if err := GenerateTokensAndSetCookies(c, user, secret); err != nil {
if err := auth.GenerateTokensAndSetCookies(c, user, secret); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to generate tokens").SetInternal(err)
}
if err := s.createUserAuthSignUpActivity(c, user); err != nil {
if err := s.createAuthSignUpActivity(c, user); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, "Failed to create activity").SetInternal(err)
}
return c.JSON(http.StatusOK, composeResponse(user))
return c.JSON(http.StatusOK, user)
})
g.POST("/auth/signout", func(c echo.Context) error {
RemoveTokensAndCookies(c)
auth.RemoveTokensAndCookies(c)
return c.JSON(http.StatusOK, true)
})
}
func (s *Server) createUserAuthSignInActivity(c echo.Context, user *api.User) error {
func (s *APIV1Service) createAuthSignInActivity(c echo.Context, user *api.User) error {
ctx := c.Request().Context()
payload := api.ActivityUserAuthSignInPayload{
UserID: user.ID,
......@@ -234,7 +250,7 @@ func (s *Server) createUserAuthSignInActivity(c echo.Context, user *api.User) er
return err
}
func (s *Server) createUserAuthSignUpActivity(c echo.Context, user *api.User) error {
func (s *APIV1Service) createAuthSignUpActivity(c echo.Context, user *api.User) error {
ctx := c.Request().Context()
payload := api.ActivityUserAuthSignUpPayload{
Username: user.Username,
......
api/v1/test.go 0 → 100644
View file @ 4ed9a3a0
package v1
import "github.com/labstack/echo/v4"
func (*APIV1Service) registerTestRoutes(g *echo.Group) {
g.GET("/test", func(c echo.Context) error {
return c.String(200, "Hello World")
})
}
api/v1/v1.go 0 → 100644
View file @ 4ed9a3a0
package v1
import (
"github.com/labstack/echo/v4"
"github.com/usememos/memos/server/profile"
"github.com/usememos/memos/store"
)
type APIV1Service struct {
Secret string
Profile *profile.Profile
Store *store.Store
}
func NewAPIV1Service(secret string, profile *profile.Profile, store *store.Store) *APIV1Service {
return &APIV1Service{
Secret: secret,
Profile: profile,
Store: store,
}
}
func (s *APIV1Service) Register(e *echo.Echo) {
apiV1Group := e.Group("/api/v1")
s.registerTestRoutes(apiV1Group)
s.registerAuthRoutes(apiV1Group, s.Secret)
}
cmd/memos.go
View file @ 4ed9a3a0
......@@ -41,7 +41,15 @@ var (
Short: `An open-source, self-hosted memo hub with knowledge management and social networking.`,
Run: func(_cmd *cobra.Command, _args []string) {
ctx, cancel := context.WithCancel(context.Background())
s, err := server.NewServer(ctx, profile)
db := db.NewDB(profile)
if err := db.Open(ctx); err != nil {
cancel()
fmt.Printf("failed to open db, error: %+v\n", err)
return
}
store := store.New(db.DBInstance, profile)
s, err := server.NewServer(ctx, profile, store)
if err != nil {
cancel()
fmt.Printf("failed to create server, error: %+v\n", err)
......
server/auth/auth.go
View file @ 4ed9a3a0
package auth
import (
"net/http"
"strconv"
"time"
"github.com/golang-jwt/jwt/v4"
"github.com/labstack/echo/v4"
"github.com/pkg/errors"
"github.com/usememos/memos/api"
)
const (
......@@ -59,6 +63,48 @@ func GenerateRefreshToken(userName string, userID int, secret string) (string, e
return generateToken(userName, userID, RefreshTokenAudienceName, expirationTime, []byte(secret))
}
// GenerateTokensAndSetCookies generates jwt token and saves it to the http-only cookie.
func GenerateTokensAndSetCookies(c echo.Context, user *api.User, secret string) error {
accessToken, err := GenerateAccessToken(user.Username, user.ID, secret)
if err != nil {
return errors.Wrap(err, "failed to generate access token")
}
cookieExp := time.Now().Add(CookieExpDuration)
setTokenCookie(c, AccessTokenCookieName, accessToken, cookieExp)
// We generate here a new refresh token and saving it to the cookie.
refreshToken, err := GenerateRefreshToken(user.Username, user.ID, secret)
if err != nil {
return errors.Wrap(err, "failed to generate refresh token")
}
setTokenCookie(c, RefreshTokenCookieName, refreshToken, cookieExp)
return nil
}
// RemoveTokensAndCookies removes the jwt token and refresh token from the cookies.
func RemoveTokensAndCookies(c echo.Context) {
// We set the expiration time to the past, so that the cookie will be removed.
cookieExp := time.Now().Add(-1 * time.Hour)
setTokenCookie(c, AccessTokenCookieName, "", cookieExp)
setTokenCookie(c, RefreshTokenCookieName, "", cookieExp)
}
// setTokenCookie sets the token to the cookie.
func setTokenCookie(c echo.Context, name, token string, expiration time.Time) {
cookie := new(http.Cookie)
cookie.Name = name
cookie.Value = token
cookie.Expires = expiration
cookie.Path = "/"
// Http-only helps mitigate the risk of client side script accessing the protected cookie.
cookie.HttpOnly = true
cookie.SameSite = http.SameSiteStrictMode
c.SetCookie(cookie)
}
// generateToken generates a jwt token.
func generateToken(username string, userID int, aud string, expirationTime time.Time, secret []byte) (string, error) {
// Create the JWT claims, which includes the username and expiry time.
claims := &claimsMessage{
......
server/common.go
View file @ 4ed9a3a0
......@@ -27,12 +27,12 @@ func defaultAPIRequestSkipper(c echo.Context) bool {
return common.HasPrefixes(path, "/api")
}
func (server *Server) defaultAuthSkipper(c echo.Context) bool {
func (s *Server) defaultAuthSkipper(c echo.Context) bool {
ctx := c.Request().Context()
path := c.Path()
// Skip auth.
if common.HasPrefixes(path, "/api/auth") {
if common.HasPrefixes(path, "/api/v1/auth") {
return true
}
......@@ -42,7 +42,7 @@ func (server *Server) defaultAuthSkipper(c echo.Context) bool {
userFind := &api.UserFind{
OpenID: &openID,
}
user, err := server.Store.FindUser(ctx, userFind)
user, err := s.Store.FindUser(ctx, userFind)
if err != nil && common.ErrorCode(err) != common.NotFound {
return false
}
......
server/jwt.go
View file @ 4ed9a3a0
......@@ -33,47 +33,6 @@ func getUserIDContextKey() string {
return userIDContextKey
}
// GenerateTokensAndSetCookies generates jwt token and saves it to the http-only cookie.
func GenerateTokensAndSetCookies(c echo.Context, user *api.User, secret string) error {
accessToken, err := auth.GenerateAccessToken(user.Username, user.ID, secret)
if err != nil {
return errors.Wrap(err, "failed to generate access token")
}
cookieExp := time.Now().Add(auth.CookieExpDuration)
setTokenCookie(c, auth.AccessTokenCookieName, accessToken, cookieExp)
// We generate here a new refresh token and saving it to the cookie.
refreshToken, err := auth.GenerateRefreshToken(user.Username, user.ID, secret)
if err != nil {
return errors.Wrap(err, "failed to generate refresh token")
}
setTokenCookie(c, auth.RefreshTokenCookieName, refreshToken, cookieExp)
return nil
}
// RemoveTokensAndCookies removes the jwt token and refresh token from the cookies.
func RemoveTokensAndCookies(c echo.Context) {
// We set the expiration time to the past, so that the cookie will be removed.
cookieExp := time.Now().Add(-1 * time.Hour)
setTokenCookie(c, auth.AccessTokenCookieName, "", cookieExp)
setTokenCookie(c, auth.RefreshTokenCookieName, "", cookieExp)
}
// Here we are creating a new cookie, which will store the valid JWT token.
func setTokenCookie(c echo.Context, name, token string, expiration time.Time) {
cookie := new(http.Cookie)
cookie.Name = name
cookie.Value = token
cookie.Expires = expiration
cookie.Path = "/"
// Http-only helps mitigate the risk of client side script accessing the protected cookie.
cookie.HttpOnly = true
cookie.SameSite = http.SameSiteStrictMode
c.SetCookie(cookie)
}
func extractTokenFromHeader(c echo.Context) (string, error) {
authHeader := c.Request().Header.Get("Authorization")
if authHeader == "" {
......@@ -101,6 +60,15 @@ func findAccessToken(c echo.Context) string {
return accessToken
}
func audienceContains(audience jwt.ClaimStrings, token string) bool {
for _, v := range audience {
if v == token {
return true
}
}
return false
}
// JWTMiddleware validates the access token.
// If the access token is about to expire or has expired and the request has a valid refresh token, it
// will try to generate new access token and refresh token.
......@@ -226,7 +194,7 @@ func JWTMiddleware(server *Server, next echo.HandlerFunc, secret string) echo.Ha
// If we have a valid refresh token, we will generate new access token and refresh token
if refreshToken != nil && refreshToken.Valid {
if err := GenerateTokensAndSetCookies(c, user, secret); err != nil {
if err := auth.GenerateTokensAndSetCookies(c, user, secret); err != nil {
return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("Server error to refresh expired token. User Id %d", userID)).SetInternal(err)
}
}
......@@ -246,12 +214,3 @@ func JWTMiddleware(server *Server, next echo.HandlerFunc, secret string) echo.Ha
return next(c)
}
}
func audienceContains(audience jwt.ClaimStrings, token string) bool {
for _, v := range audience {
if v == token {
return true
}
}
return false
}
server/server.go
View file @ 4ed9a3a0
......@@ -2,53 +2,45 @@ package server
import (
"context"
"database/sql"
"encoding/json"
"fmt"
"time"
"github.com/pkg/errors"
"github.com/usememos/memos/api"
apiV1 "github.com/usememos/memos/api/v1"
"github.com/usememos/memos/plugin/telegram"
"github.com/usememos/memos/server/profile"
"github.com/usememos/memos/store"
"github.com/usememos/memos/store/db"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
)
type Server struct {
e *echo.Echo
db *sql.DB
e *echo.Echo
ID string
Secret string
Profile *profile.Profile
Store *store.Store
telegramBot *telegram.Bot
}
func NewServer(ctx context.Context, profile *profile.Profile) (*Server, error) {
func NewServer(ctx context.Context, profile *profile.Profile, store *store.Store) (*Server, error) {
e := echo.New()
e.Debug = true
e.HideBanner = true
e.HidePort = true
db := db.NewDB(profile)
if err := db.Open(ctx); err != nil {
return nil, errors.Wrap(err, "cannot open db")
}
s := &Server{
e: e,
db: db.DBInstance,
Store: store,
Profile: profile,
}
storeInstance := store.New(db.DBInstance, profile)
s.Store = storeInstance
telegramBotHandler := newTelegramHandler(storeInstance)
telegramBotHandler := newTelegramHandler(store)
s.telegramBot = telegram.NewBotWithHandler(telegramBotHandler)
e.Use(middleware.LoggerWithConfig(middleware.LoggerConfig{
......@@ -89,23 +81,23 @@ func NewServer(ctx context.Context, profile *profile.Profile) (*Server, error) {
return nil, err
}
}
s.Secret = secret
rootGroup := e.Group("")
s.registerRSSRoutes(rootGroup)
publicGroup := e.Group("/o")
publicGroup.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
return JWTMiddleware(s, next, secret)
return JWTMiddleware(s, next, s.Secret)
})
registerGetterPublicRoutes(publicGroup)
s.registerResourcePublicRoutes(publicGroup)
apiGroup := e.Group("/api")
apiGroup.Use(func(next echo.HandlerFunc) echo.HandlerFunc {
return JWTMiddleware(s, next, secret)
return JWTMiddleware(s, next, s.Secret)
})
s.registerSystemRoutes(apiGroup)
s.registerAuthRoutes(apiGroup, secret)
s.registerUserRoutes(apiGroup)
s.registerMemoRoutes(apiGroup)
s.registerMemoResourceRoutes(apiGroup)
......@@ -117,6 +109,9 @@ func NewServer(ctx context.Context, profile *profile.Profile) (*Server, error) {
s.registerOpenAIRoutes(apiGroup)
s.registerMemoRelationRoutes(apiGroup)
apiV1Service := apiV1.NewAPIV1Service(s.Secret, profile, store)
apiV1Service.Register(e)
return s, nil
}
......@@ -140,7 +135,7 @@ func (s *Server) Shutdown(ctx context.Context) {
}
// Close database connection
if err := s.db.Close(); err != nil {
if err := s.Store.GetDB().Close(); err != nil {
fmt.Printf("failed to close database, error: %v\n", err)
}
......
store/store.go
View file @ 4ed9a3a0
......@@ -28,6 +28,10 @@ func New(db *sql.DB, profile *profile.Profile) *Store {
}
}
func (s *Store) GetDB() *sql.DB {
return s.db
}
func (s *Store) Vacuum(ctx context.Context) error {
tx, err := s.db.BeginTx(ctx, nil)
if err != nil {
......
test/server/auth_test.go
View file @ 4ed9a3a0
......@@ -9,6 +9,7 @@ import (
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/usememos/memos/api"
apiv1 "github.com/usememos/memos/api/v1"
)
func TestAuthServer(t *testing.T) {
......@@ -17,7 +18,7 @@ func TestAuthServer(t *testing.T) {
require.NoError(t, err)
defer s.Shutdown(ctx)
signup := &api.SignUp{
signup := &apiv1.SignUp{
Username: "testuser",
Password: "testpassword",
}
......@@ -26,15 +27,15 @@ func TestAuthServer(t *testing.T) {
require.Equal(t, signup.Username, user.Username)
}
func (s *TestingServer) postAuthSignup(signup *api.SignUp) (*api.User, error) {
func (s *TestingServer) postAuthSignup(signup *apiv1.SignUp) (*api.User, error) {
rawData, err := json.Marshal(&signup)
if err != nil {
return nil, errors.Wrap(err, "failed to marshal signup")
}
reader := bytes.NewReader(rawData)
body, err := s.post("/api/auth/signup", reader, nil)
body, err := s.post("/api/v1/auth/signup", reader, nil)
if err != nil {
return nil, err
return nil, errors.Wrap(err, "fail to post request")
}
buf := &bytes.Buffer{}
......@@ -43,12 +44,9 @@ func (s *TestingServer) postAuthSignup(signup *api.SignUp) (*api.User, error) {
return nil, errors.Wrap(err, "fail to read response body")
}
type AuthSignupResponse struct {
Data *api.User `json:"data"`
}
res := new(AuthSignupResponse)
if err = json.Unmarshal(buf.Bytes(), res); err != nil {
user := &api.User{}
if err = json.Unmarshal(buf.Bytes(), user); err != nil {
return nil, errors.Wrap(err, "fail to unmarshal post signup response")
}
return res.Data, nil
return user, nil
}
test/server/memo_relation_test.go
View file @ 4ed9a3a0
......@@ -10,6 +10,7 @@ import (
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/usememos/memos/api"
apiv1 "github.com/usememos/memos/api/v1"
)
func TestMemoRelationServer(t *testing.T) {
......@@ -18,7 +19,7 @@ func TestMemoRelationServer(t *testing.T) {
require.NoError(t, err)
defer s.Shutdown(ctx)
signup := &api.SignUp{
signup := &apiv1.SignUp{
Username: "testuser",
Password: "testpassword",
}
......
test/server/memo_test.go
View file @ 4ed9a3a0
......@@ -10,6 +10,7 @@ import (
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/usememos/memos/api"
apiv1 "github.com/usememos/memos/api/v1"
)
func TestMemoServer(t *testing.T) {
......@@ -18,7 +19,7 @@ func TestMemoServer(t *testing.T) {
require.NoError(t, err)
defer s.Shutdown(ctx)
signup := &api.SignUp{
signup := &apiv1.SignUp{
Username: "testuser",
Password: "testpassword",
}
......
test/server/server.go
View file @ 4ed9a3a0
......@@ -13,6 +13,7 @@ import (
"github.com/pkg/errors"
"github.com/usememos/memos/server"
"github.com/usememos/memos/server/profile"
"github.com/usememos/memos/store"
"github.com/usememos/memos/store/db"
"github.com/usememos/memos/test"
......@@ -34,19 +35,19 @@ func NewTestingServer(ctx context.Context, t *testing.T) (*TestingServer, error)
return nil, errors.Wrap(err, "failed to open db")
}
server, err := server.NewServer(ctx, profile)
store := store.New(db.DBInstance, profile)
server, err := server.NewServer(ctx, profile, store)
if err != nil {
return nil, errors.Wrap(err, "failed to create server")
}
errChan := make(chan error, 1)
s := &TestingServer{
server: server,
client: &http.Client{},
profile: profile,
cookie: "",
}
errChan := make(chan error, 1)
go func() {
if err := s.server.Start(ctx); err != nil {
......@@ -126,7 +127,7 @@ func (s *TestingServer) request(method, uri string, body io.Reader, params, head
}
if method == "POST" {
if strings.Contains(uri, "/api/auth/login") || strings.Contains(uri, "/api/auth/signup") {
if strings.Contains(uri, "/api/v1/auth/login") || strings.Contains(uri, "/api/v1/auth/signup") {
cookie := ""
h := resp.Header.Get("Set-Cookie")
parts := strings.Split(h, "; ")
......@@ -140,7 +141,7 @@ func (s *TestingServer) request(method, uri string, body io.Reader, params, head
return nil, errors.Errorf("unable to find access token in the login response headers")
}
s.cookie = cookie
} else if strings.Contains(uri, "/api/auth/logout") {
} else if strings.Contains(uri, "/api/v1/auth/logout") {
s.cookie = ""
}
}
......
test/server/system_test.go
View file @ 4ed9a3a0
......@@ -9,6 +9,7 @@ import (
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/usememos/memos/api"
apiv1 "github.com/usememos/memos/api/v1"
)
func TestSystemServer(t *testing.T) {
......@@ -21,7 +22,7 @@ func TestSystemServer(t *testing.T) {
require.NoError(t, err)
require.Equal(t, (*api.User)(nil), status.Host)
signup := &api.SignUp{
signup := &apiv1.SignUp{
Username: "testuser",
Password: "testpassword",
}
......
web/src/helpers/api.ts
View file @ 4ed9a3a0
......@@ -23,14 +23,14 @@ export function vacuumDatabase() {
}
export function signin(username: string, password: string) {
return axios.post<ResponseObject<User>>("/api/auth/signin", {
return axios.post("/api/v1/auth/signin", {
username,
password,
});
}
export function signinWithSSO(identityProviderId: IdentityProviderId, code: string, redirectUri: string) {
return axios.post<ResponseObject<User>>("/api/auth/signin/sso", {
return axios.post("/api/v1/auth/signin/sso", {
identityProviderId,
code,
redirectUri,
......@@ -38,14 +38,14 @@ export function signinWithSSO(identityProviderId: IdentityProviderId, code: stri
}
export function signup(username: string, password: string) {
return axios.post<ResponseObject<User>>("/api/auth/signup", {
return axios.post("/api/v1/auth/signup", {
username,
password,
});
}
export function signout() {
return axios.post("/api/auth/signout");
return axios.post("/api/v1/auth/signout");
}
export function createUser(userCreate: UserCreate) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment