From 664927ea58fa74386737ea83da267b3f7edd6566 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Sun, 30 Jan 2022 15:01:04 +0100 Subject: Bump nanoid from 3.1.25 to 3.2.0 in /web/gotosocial-styling (#364) Bumps [nanoid](https://github.com/ai/nanoid) from 3.1.25 to 3.2.0. - [Release notes](https://github.com/ai/nanoid/releases) - [Changelog](https://github.com/ai/nanoid/blob/main/CHANGELOG.md) - [Commits](https://github.com/ai/nanoid/compare/3.1.25...3.2.0) --- updated-dependencies: - dependency-name: nanoid dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- web/gotosocial-styling/yarn.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'web') diff --git a/web/gotosocial-styling/yarn.lock b/web/gotosocial-styling/yarn.lock index 1d39d5fd7..6b7fe6e64 100644 --- a/web/gotosocial-styling/yarn.lock +++ b/web/gotosocial-styling/yarn.lock @@ -230,9 +230,9 @@ ms@^2.1.1: integrity sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA== nanoid@^3.1.23: - version "3.1.25" - resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.1.25.tgz#09ca32747c0e543f0e1814b7d3793477f9c8e152" - integrity sha512-rdwtIXaXCLFAQbnfqDRnI6jaRHp9fTcYBjtFKE8eezcZ7LuLjhUaQGNeMXf1HmRoCH32CLz6XwX0TtxEOS/A3Q== + version "3.2.0" + resolved "https://registry.yarnpkg.com/nanoid/-/nanoid-3.2.0.tgz#62667522da6673971cca916a6d3eff3f415ff80c" + integrity sha512-fmsZYa9lpn69Ad5eDn7FMcnnSR+8R34W9qJEijxYhTbfOWzr22n1QxCMzXLK+ODyW2973V3Fux959iQoUxzUIA== normalize-path@^3.0.0, normalize-path@~3.0.0: version "3.0.0" -- cgit v1.3 From 5be8a7a7ea96d962d0f0b9f09b967e403a227698 Mon Sep 17 00:00:00 2001 From: Forest Johnson Date: Mon, 31 Jan 2022 10:46:20 +0000 Subject: [bug] Send plaintext emails to fix "message refused: Message is not RFC 2822 compliant" (#366) * trying to fix "message refused: Message is not RFC 2822 compliant" * fix "message refused: Message is not RFC 2822 compliant" 550 5.7.1 Delivery not authorized, message refused: Message is not RFC 2822 compliant * remove silly regex * lint * fix tests * we should use text/template instead of html/template now --- internal/email/confirm.go | 14 +++++++-- internal/email/noopsender.go | 12 +++++-- internal/email/reset.go | 9 ++++-- internal/email/sender.go | 2 +- internal/email/util.go | 45 ++++++++++++++++++--------- internal/email/util_test.go | 4 +-- internal/processing/user/emailconfirm_test.go | 2 +- web/template/email_confirm.tmpl | 29 ----------------- web/template/email_confirm_html.tmpl | 29 +++++++++++++++++ web/template/email_confirm_text.tmpl | 9 ++++++ web/template/email_reset.tmpl | 29 ----------------- web/template/email_reset_html.tmpl | 29 +++++++++++++++++ web/template/email_reset_text.tmpl | 9 ++++++ 13 files changed, 136 insertions(+), 86 deletions(-) delete mode 100644 web/template/email_confirm.tmpl create mode 100644 web/template/email_confirm_html.tmpl create mode 100644 web/template/email_confirm_text.tmpl delete mode 100644 web/template/email_reset.tmpl create mode 100644 web/template/email_reset_html.tmpl create mode 100644 web/template/email_reset_text.tmpl (limited to 'web') diff --git a/internal/email/confirm.go b/internal/email/confirm.go index 4503137b3..34e2fb660 100644 --- a/internal/email/confirm.go +++ b/internal/email/confirm.go @@ -21,11 +21,15 @@ package email import ( "bytes" "net/smtp" + + "github.com/sirupsen/logrus" + "github.com/spf13/viper" + "github.com/superseriousbusiness/gotosocial/internal/config" ) const ( - confirmTemplate = "email_confirm.tmpl" - confirmSubject = "Subject: GoToSocial Email Confirmation" + confirmTemplate = "email_confirm_text.tmpl" + confirmSubject = "GoToSocial Email Confirmation" ) func (s *sender) SendConfirmEmail(toAddress string, data ConfirmData) error { @@ -35,7 +39,11 @@ func (s *sender) SendConfirmEmail(toAddress string, data ConfirmData) error { } confirmBody := buf.String() - msg := assembleMessage(confirmSubject, confirmBody, toAddress, s.from) + msg, err := assembleMessage(confirmSubject, confirmBody, toAddress, s.from) + if err != nil { + return err + } + logrus.WithField("func", "SendConfirmEmail").Trace(s.hostAddress + "\n" + viper.GetString(config.Keys.SMTPUsername) + ":password" + "\n" + s.from + "\n" + toAddress + "\n\n" + string(msg) + "\n") return smtp.SendMail(s.hostAddress, s.auth, s.from, []string{toAddress}, msg) } diff --git a/internal/email/noopsender.go b/internal/email/noopsender.go index efec303f0..9f587f319 100644 --- a/internal/email/noopsender.go +++ b/internal/email/noopsender.go @@ -20,7 +20,7 @@ package email import ( "bytes" - "html/template" + "text/template" "github.com/sirupsen/logrus" "github.com/spf13/viper" @@ -57,7 +57,10 @@ func (s *noopSender) SendConfirmEmail(toAddress string, data ConfirmData) error } confirmBody := buf.String() - msg := assembleMessage(confirmSubject, confirmBody, toAddress, "test@example.org") + msg, err := assembleMessage(confirmSubject, confirmBody, toAddress, "test@example.org") + if err != nil { + return err + } logrus.Tracef("NOT SENDING confirmation email to %s with contents: %s", toAddress, msg) @@ -74,7 +77,10 @@ func (s *noopSender) SendResetEmail(toAddress string, data ResetData) error { } resetBody := buf.String() - msg := assembleMessage(resetSubject, resetBody, toAddress, "test@example.org") + msg, err := assembleMessage(resetSubject, resetBody, toAddress, "test@example.org") + if err != nil { + return err + } logrus.Tracef("NOT SENDING reset email to %s with contents: %s", toAddress, msg) diff --git a/internal/email/reset.go b/internal/email/reset.go index 7a08ebda9..b646ef99b 100644 --- a/internal/email/reset.go +++ b/internal/email/reset.go @@ -24,8 +24,8 @@ import ( ) const ( - resetTemplate = "email_reset.tmpl" - resetSubject = "Subject: GoToSocial Password Reset" + resetTemplate = "email_reset_text.tmpl" + resetSubject = "GoToSocial Password Reset" ) func (s *sender) SendResetEmail(toAddress string, data ResetData) error { @@ -35,7 +35,10 @@ func (s *sender) SendResetEmail(toAddress string, data ResetData) error { } resetBody := buf.String() - msg := assembleMessage(resetSubject, resetBody, toAddress, s.from) + msg, err := assembleMessage(resetSubject, resetBody, toAddress, s.from) + if err != nil { + return err + } return smtp.SendMail(s.hostAddress, s.auth, s.from, []string{toAddress}, msg) } diff --git a/internal/email/sender.go b/internal/email/sender.go index 97bbcd23b..f44627496 100644 --- a/internal/email/sender.go +++ b/internal/email/sender.go @@ -20,8 +20,8 @@ package email import ( "fmt" - "html/template" "net/smtp" + "text/template" "github.com/spf13/viper" "github.com/superseriousbusiness/gotosocial/internal/config" diff --git a/internal/email/util.go b/internal/email/util.go index db95128fa..52290dbe4 100644 --- a/internal/email/util.go +++ b/internal/email/util.go @@ -19,15 +19,12 @@ package email import ( + "errors" "fmt" - "html/template" "os" "path/filepath" -) - -const ( - mime = `MIME-version: 1.0; -Content-Type: text/html;` + "strings" + "text/template" ) func loadTemplates(templateBaseDir string) (*template.Template, error) { @@ -41,16 +38,34 @@ func loadTemplates(templateBaseDir string) (*template.Template, error) { return template.ParseGlob(tmPath) } -func assembleMessage(mailSubject string, mailBody string, mailTo string, mailFrom string) []byte { - from := fmt.Sprintf("From: GoToSocial <%s>", mailFrom) - to := fmt.Sprintf("To: %s", mailTo) +// https://datatracker.ietf.org/doc/html/rfc2822 +// I did not read the RFC, I just copy and pasted from +// https://pkg.go.dev/net/smtp#SendMail +// and it did seem to work. +func assembleMessage(mailSubject string, mailBody string, mailTo string, mailFrom string) ([]byte, error) { + + if strings.Contains(mailSubject, "\r") || strings.Contains(mailSubject, "\n") { + return nil, errors.New("email subject must not contain newline characters") + } + + if strings.Contains(mailFrom, "\r") || strings.Contains(mailFrom, "\n") { + return nil, errors.New("email from address must not contain newline characters") + } + + if strings.Contains(mailTo, "\r") || strings.Contains(mailTo, "\n") { + return nil, errors.New("email to address must not contain newline characters") + } + + // normalize the message body to use CRLF line endings + mailBody = strings.ReplaceAll(mailBody, "\r\n", "\n") + mailBody = strings.ReplaceAll(mailBody, "\n", "\r\n") msg := []byte( - mailSubject + "\r\n" + - from + "\r\n" + - to + "\r\n" + - mime + "\r\n" + - mailBody + "\r\n") + "To: " + mailTo + "\r\n" + + "Subject: " + mailSubject + "\r\n" + + "\r\n" + + mailBody + "\r\n", + ) - return msg + return msg, nil } diff --git a/internal/email/util_test.go b/internal/email/util_test.go index b5c7a9852..8895785f7 100644 --- a/internal/email/util_test.go +++ b/internal/email/util_test.go @@ -39,7 +39,7 @@ func (suite *UtilTestSuite) TestTemplateConfirm() { suite.sender.SendConfirmEmail("user@example.org", confirmData) suite.Len(suite.sentEmails, 1) - suite.Equal("Subject: GoToSocial Email Confirmation\r\nFrom: GoToSocial \r\nTo: user@example.org\r\nMIME-version: 1.0;\nContent-Type: text/html;\r\n\n\n \n \n
\n

\n Hello test!\n

\n
\n
\n

\n You are receiving this mail because you've requested an account on Test Instance.\n

\n

\n We just need to confirm that this is your email address. To confirm your email, click here or paste the following in your browser's address bar:\n

\n

\n \n https://example.org/confirm_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\n \n

\n
\n
\n

\n If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of Test Instance.\n

\n
\n \n\r\n", suite.sentEmails["user@example.org"]) + suite.Equal("To: user@example.org\r\nSubject: GoToSocial Email Confirmation\r\n\r\nHello test!\r\n\r\nYou are receiving this mail because you've requested an account on https://example.org.\r\n\r\nWe just need to confirm that this is your email address. To confirm your email, paste the following in your browser's address bar:\r\n\r\nhttps://example.org/confirm_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\r\n\r\nIf you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of https://example.org\r\n\r\n", suite.sentEmails["user@example.org"]) } func (suite *UtilTestSuite) TestTemplateReset() { @@ -52,7 +52,7 @@ func (suite *UtilTestSuite) TestTemplateReset() { suite.sender.SendResetEmail("user@example.org", resetData) suite.Len(suite.sentEmails, 1) - suite.Equal("Subject: GoToSocial Password Reset\r\nFrom: GoToSocial \r\nTo: user@example.org\r\nMIME-version: 1.0;\nContent-Type: text/html;\r\n\n\n \n \n
\n

\n Hello test!\n

\n
\n
\n

\n You are receiving this mail because a password reset has been requested for your account on Test Instance.\n

\n

\n To reset your password, click here or paste the following in your browser's address bar:\n

\n

\n \n https://example.org/reset_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\n \n

\n
\n
\n

\n If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of Test Instance.\n

\n
\n \n\r\n", suite.sentEmails["user@example.org"]) + suite.Equal("To: user@example.org\r\nSubject: GoToSocial Password Reset\r\n\r\nHello test!\r\n\r\nYou are receiving this mail because a password reset has been requested for your account on https://example.org.\r\n\r\nTo reset your password, paste the following in your browser's address bar:\r\n\r\nhttps://example.org/reset_email?token=ee24f71d-e615-43f9-afae-385c0799b7fa\r\n\r\nIf you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of https://example.org.\r\n\r\n", suite.sentEmails["user@example.org"]) } func TestUtilTestSuite(t *testing.T) { diff --git a/internal/processing/user/emailconfirm_test.go b/internal/processing/user/emailconfirm_test.go index 58836d40d..6f22306a1 100644 --- a/internal/processing/user/emailconfirm_test.go +++ b/internal/processing/user/emailconfirm_test.go @@ -54,7 +54,7 @@ func (suite *EmailConfirmTestSuite) TestSendConfirmEmail() { suite.NotEmpty(token) // email should contain the token - emailShould := fmt.Sprintf("Subject: GoToSocial Email Confirmation\r\nFrom: GoToSocial \r\nTo: some.email@example.org\r\nMIME-version: 1.0;\nContent-Type: text/html;\r\n\n\n \n \n
\n

\n Hello the_mighty_zork!\n

\n
\n
\n

\n You are receiving this mail because you've requested an account on localhost:8080.\n

\n

\n We just need to confirm that this is your email address. To confirm your email, click here or paste the following in your browser's address bar:\n

\n

\n \n http://localhost:8080/confirm_email?token=%s\n \n

\n
\n
\n

\n If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of localhost:8080.\n

\n
\n \n\r\n", token, token) + emailShould := fmt.Sprintf("To: some.email@example.org\r\nSubject: GoToSocial Email Confirmation\r\n\r\nHello the_mighty_zork!\r\n\r\nYou are receiving this mail because you've requested an account on http://localhost:8080.\r\n\r\nWe just need to confirm that this is your email address. To confirm your email, paste the following in your browser's address bar:\r\n\r\nhttp://localhost:8080/confirm_email?token=%s\r\n\r\nIf you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of http://localhost:8080\r\n\r\n", token) suite.Equal(emailShould, email) // confirmationSentAt should be recent diff --git a/web/template/email_confirm.tmpl b/web/template/email_confirm.tmpl deleted file mode 100644 index 0a9907921..000000000 --- a/web/template/email_confirm.tmpl +++ /dev/null @@ -1,29 +0,0 @@ - - - - -
-

- Hello {{.Username}}! -

-
-
-

- You are receiving this mail because you've requested an account on {{.InstanceName}}. -

-

- We just need to confirm that this is your email address. To confirm your email, click here or paste the following in your browser's address bar: -

-

- - {{.ConfirmLink}} - -

-
-
-

- If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceName}}. -

-
- - \ No newline at end of file diff --git a/web/template/email_confirm_html.tmpl b/web/template/email_confirm_html.tmpl new file mode 100644 index 000000000..0a9907921 --- /dev/null +++ b/web/template/email_confirm_html.tmpl @@ -0,0 +1,29 @@ + + + + +
+

+ Hello {{.Username}}! +

+
+
+

+ You are receiving this mail because you've requested an account on {{.InstanceName}}. +

+

+ We just need to confirm that this is your email address. To confirm your email, click here or paste the following in your browser's address bar: +

+

+ + {{.ConfirmLink}} + +

+
+
+

+ If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceName}}. +

+
+ + \ No newline at end of file diff --git a/web/template/email_confirm_text.tmpl b/web/template/email_confirm_text.tmpl new file mode 100644 index 000000000..8682cc49c --- /dev/null +++ b/web/template/email_confirm_text.tmpl @@ -0,0 +1,9 @@ +Hello {{.Username}}! + +You are receiving this mail because you've requested an account on {{.InstanceURL}}. + +We just need to confirm that this is your email address. To confirm your email, paste the following in your browser's address bar: + +{{.ConfirmLink}} + +If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceURL}} diff --git a/web/template/email_reset.tmpl b/web/template/email_reset.tmpl deleted file mode 100644 index 7318c6a45..000000000 --- a/web/template/email_reset.tmpl +++ /dev/null @@ -1,29 +0,0 @@ - - - - -
-

- Hello {{.Username}}! -

-
-
-

- You are receiving this mail because a password reset has been requested for your account on {{.InstanceName}}. -

-

- To reset your password, click here or paste the following in your browser's address bar: -

-

- - {{.ResetLink}} - -

-
-
-

- If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceName}}. -

-
- - \ No newline at end of file diff --git a/web/template/email_reset_html.tmpl b/web/template/email_reset_html.tmpl new file mode 100644 index 000000000..7318c6a45 --- /dev/null +++ b/web/template/email_reset_html.tmpl @@ -0,0 +1,29 @@ + + + + +
+

+ Hello {{.Username}}! +

+
+
+

+ You are receiving this mail because a password reset has been requested for your account on {{.InstanceName}}. +

+

+ To reset your password, click here or paste the following in your browser's address bar: +

+

+ + {{.ResetLink}} + +

+
+
+

+ If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceName}}. +

+
+ + \ No newline at end of file diff --git a/web/template/email_reset_text.tmpl b/web/template/email_reset_text.tmpl new file mode 100644 index 000000000..ccac3bf75 --- /dev/null +++ b/web/template/email_reset_text.tmpl @@ -0,0 +1,9 @@ +Hello {{.Username}}! + +You are receiving this mail because a password reset has been requested for your account on {{.InstanceURL}}. + +To reset your password, paste the following in your browser's address bar: + +{{.ResetLink}} + +If you believe you've been sent this email in error, feel free to ignore it, or contact the administrator of {{.InstanceURL}}. -- cgit v1.3 From 6ed368cbebcae087aec1f31ee8d69ac6c47ead9f Mon Sep 17 00:00:00 2001 From: Forest Johnson Date: Mon, 7 Feb 2022 11:04:31 +0000 Subject: [feature] add authorization to the already-existing authentication (#365) * add ensureUserIsAuthorizedOrRedirect to /oauth/authorize * adding authorization (email confirm, account approve, etc) to TokenCheck * revert un-needed changes to signin.go * oops what happened here * error css * add account.SuspendedAt check * remove redundant checks from oauth util Authed function * wip tests * tests passing * stop stripping useful information from ErrAlreadyExists * that feeling of scraping the dryer LINT off the screen * oops I didn't mean to get rid of this NewTestRouter function * make tests work with recorder * re-add ConfigureTemplatesWithGin to handle template path err Co-authored-by: tsmethurst --- internal/api/client/auth/auth.go | 13 +++ internal/api/client/auth/auth_test.go | 94 +++++++++++++++++++- internal/api/client/auth/authorize.go | 73 ++++++++++++--- internal/api/client/auth/authorize_test.go | 113 ++++++++++++++++++++++++ internal/api/security/tokencheck.go | 22 +++++ internal/db/bundb/errors.go | 4 +- internal/db/error.go | 15 +++- internal/federation/dereferencing/attachment.go | 4 +- internal/federation/federatingdb/create.go | 3 +- internal/oauth/util.go | 21 +---- internal/processing/status/util.go | 7 +- internal/router/router.go | 2 +- internal/router/session.go | 6 +- internal/router/template.go | 11 ++- testrig/router.go | 30 +++++++ web/assets/base.css | 19 ++++ web/gotosocial-styling/templates/base.css | 18 ++++ web/template/authorize.tmpl | 8 +- web/template/error.tmpl | 8 ++ 19 files changed, 424 insertions(+), 47 deletions(-) create mode 100644 internal/api/client/auth/authorize_test.go create mode 100644 web/template/error.tmpl (limited to 'web') diff --git a/internal/api/client/auth/auth.go b/internal/api/client/auth/auth.go index 67643244b..717d997a3 100644 --- a/internal/api/client/auth/auth.go +++ b/internal/api/client/auth/auth.go @@ -32,10 +32,23 @@ import ( const ( // AuthSignInPath is the API path for users to sign in through AuthSignInPath = "/auth/sign_in" + + // CheckYourEmailPath users land here after registering a new account, instructs them to confirm thier email + CheckYourEmailPath = "/check_your_email" + + // WaitForApprovalPath users land here after confirming thier email but before an admin approves thier account + // (if such is required) + WaitForApprovalPath = "/wait_for_approval" + + // AccountDisabledPath users land here when thier account is suspended by an admin + AccountDisabledPath = "/account_disabled" + // OauthTokenPath is the API path to use for granting token requests to users with valid credentials OauthTokenPath = "/oauth/token" + // OauthAuthorizePath is the API path for authorization requests (eg., authorize this app to act on my behalf as a user) OauthAuthorizePath = "/oauth/authorize" + // CallbackPath is the API path for receiving callback tokens from external OIDC providers CallbackPath = oidc.CallbackPath diff --git a/internal/api/client/auth/auth_test.go b/internal/api/client/auth/auth_test.go index a0ee8892d..fdf1b6baf 100644 --- a/internal/api/client/auth/auth_test.go +++ b/internal/api/client/auth/auth_test.go @@ -18,4 +18,96 @@ package auth_test -// TODO +import ( + "context" + "fmt" + "net/http/httptest" + + "github.com/gin-contrib/sessions" + "github.com/gin-contrib/sessions/memstore" + "github.com/gin-gonic/gin" + "github.com/spf13/viper" + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/api/client/auth" + "github.com/superseriousbusiness/gotosocial/internal/config" + "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" + "github.com/superseriousbusiness/gotosocial/internal/oauth" + "github.com/superseriousbusiness/gotosocial/internal/oidc" + "github.com/superseriousbusiness/gotosocial/internal/router" + "github.com/superseriousbusiness/gotosocial/testrig" +) + +type AuthStandardTestSuite struct { + suite.Suite + db db.DB + idp oidc.IDP + oauthServer oauth.Server + + // standard suite models + testTokens map[string]*gtsmodel.Token + testClients map[string]*gtsmodel.Client + testApplications map[string]*gtsmodel.Application + testUsers map[string]*gtsmodel.User + testAccounts map[string]*gtsmodel.Account + + // module being tested + authModule *auth.Module +} + +const ( + sessionUserID = "userid" + sessionClientID = "client_id" +) + +func (suite *AuthStandardTestSuite) SetupSuite() { + suite.testTokens = testrig.NewTestTokens() + suite.testClients = testrig.NewTestClients() + suite.testApplications = testrig.NewTestApplications() + suite.testUsers = testrig.NewTestUsers() + suite.testAccounts = testrig.NewTestAccounts() +} + +func (suite *AuthStandardTestSuite) SetupTest() { + testrig.InitTestConfig() + suite.db = testrig.NewTestDB() + testrig.InitTestLog() + + suite.oauthServer = testrig.NewTestOauthServer(suite.db) + var err error + suite.idp, err = oidc.NewIDP(context.Background()) + if err != nil { + panic(err) + } + suite.authModule = auth.New(suite.db, suite.oauthServer, suite.idp).(*auth.Module) + testrig.StandardDBSetup(suite.db, nil) +} + +func (suite *AuthStandardTestSuite) TearDownTest() { + testrig.StandardDBTeardown(suite.db) +} + +func (suite *AuthStandardTestSuite) newContext(requestMethod string, requestPath string) (*gin.Context, *httptest.ResponseRecorder) { + // create the recorder and gin test context + recorder := httptest.NewRecorder() + ctx, engine := gin.CreateTestContext(recorder) + + // load templates into the engine + testrig.ConfigureTemplatesWithGin(engine) + + // create the request + protocol := viper.GetString(config.Keys.Protocol) + host := viper.GetString(config.Keys.Host) + baseURI := fmt.Sprintf("%s://%s", protocol, host) + requestURI := fmt.Sprintf("%s/%s", baseURI, requestPath) + ctx.Request = httptest.NewRequest(requestMethod, requestURI, nil) // the endpoint we're hitting + ctx.Request.Header.Set("accept", "text/html") + + // trigger the session middleware on the context + store := memstore.NewStore(make([]byte, 32), make([]byte, 32)) + store.Options(router.SessionOptions()) + sessionMiddleware := sessions.Sessions("gotosocial-localhost", store) + sessionMiddleware(ctx) + + return ctx, recorder +} diff --git a/internal/api/client/auth/authorize.go b/internal/api/client/auth/authorize.go index 99f3cca68..387b83c1e 100644 --- a/internal/api/client/auth/authorize.go +++ b/internal/api/client/auth/authorize.go @@ -44,7 +44,7 @@ func (m *Module) AuthorizeGETHandler(c *gin.Context) { s := sessions.Default(c) if _, err := api.NegotiateAccept(c, api.HTMLAcceptHeaders...); err != nil { - c.JSON(http.StatusNotAcceptable, gin.H{"error": err.Error()}) + c.HTML(http.StatusNotAcceptable, "error.tmpl", gin.H{"error": err.Error()}) return } @@ -57,7 +57,7 @@ func (m *Module) AuthorizeGETHandler(c *gin.Context) { if err := c.Bind(form); err != nil { l.Debugf("invalid auth form: %s", err) m.clearSession(s) - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.HTML(http.StatusBadRequest, "error.tmpl", gin.H{"error": err.Error()}) return } l.Debugf("parsed auth form: %+v", form) @@ -65,7 +65,7 @@ func (m *Module) AuthorizeGETHandler(c *gin.Context) { if err := extractAuthForm(s, form); err != nil { l.Debugf(fmt.Sprintf("error parsing form at /oauth/authorize: %s", err)) m.clearSession(s) - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.HTML(http.StatusBadRequest, "error.tmpl", gin.H{"error": err.Error()}) return } c.Redirect(http.StatusSeeOther, AuthSignInPath) @@ -75,28 +75,33 @@ func (m *Module) AuthorizeGETHandler(c *gin.Context) { // We can use the client_id on the session to retrieve info about the app associated with the client_id clientID, ok := s.Get(sessionClientID).(string) if !ok || clientID == "" { - c.JSON(http.StatusInternalServerError, gin.H{"error": "no client_id found in session"}) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": "no client_id found in session"}) return } app := >smodel.Application{} if err := m.db.GetWhere(c.Request.Context(), []db.Where{{Key: sessionClientID, Value: clientID}}, app); err != nil { m.clearSession(s) - c.JSON(http.StatusInternalServerError, gin.H{"error": fmt.Sprintf("no application found for client id %s", clientID)}) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{ + "error": fmt.Sprintf("no application found for client id %s", clientID), + }) return } - // we can also use the userid of the user to fetch their username from the db to greet them nicely <3 + // redirect the user if they have not confirmed their email yet, thier account has not been approved yet, + // or thier account has been disabled. user := >smodel.User{} if err := m.db.GetByID(c.Request.Context(), userID, user); err != nil { m.clearSession(s) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": err.Error()}) return } - acct, err := m.db.GetAccountByID(c.Request.Context(), user.AccountID) if err != nil { m.clearSession(s) - c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": err.Error()}) + return + } + if !ensureUserIsAuthorizedOrRedirect(c, user, acct) { return } @@ -104,13 +109,13 @@ func (m *Module) AuthorizeGETHandler(c *gin.Context) { redirect, ok := s.Get(sessionRedirectURI).(string) if !ok || redirect == "" { m.clearSession(s) - c.JSON(http.StatusInternalServerError, gin.H{"error": "no redirect_uri found in session"}) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": "no redirect_uri found in session"}) return } scope, ok := s.Get(sessionScope).(string) if !ok || scope == "" { m.clearSession(s) - c.JSON(http.StatusInternalServerError, gin.H{"error": "no scope found in session"}) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": "no scope found in session"}) return } @@ -170,10 +175,28 @@ func (m *Module) AuthorizePOSTHandler(c *gin.Context) { errs = append(errs, "session missing userid") } + // redirect the user if they have not confirmed their email yet, thier account has not been approved yet, + // or thier account has been disabled. + user := >smodel.User{} + if err := m.db.GetByID(c.Request.Context(), userID, user); err != nil { + m.clearSession(s) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": err.Error()}) + return + } + acct, err := m.db.GetAccountByID(c.Request.Context(), user.AccountID) + if err != nil { + m.clearSession(s) + c.HTML(http.StatusInternalServerError, "error.tmpl", gin.H{"error": err.Error()}) + return + } + if !ensureUserIsAuthorizedOrRedirect(c, user, acct) { + return + } + m.clearSession(s) if len(errs) != 0 { - c.JSON(http.StatusBadRequest, gin.H{"error": strings.Join(errs, ": ")}) + c.HTML(http.StatusBadRequest, "error.tmpl", gin.H{"error": strings.Join(errs, ": ")}) return } @@ -190,7 +213,7 @@ func (m *Module) AuthorizePOSTHandler(c *gin.Context) { // and proceed with authorization using the oauth2 library if err := m.server.HandleAuthorizeRequest(c.Writer, c.Request); err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + c.HTML(http.StatusBadRequest, "error.tmpl", gin.H{"error": err.Error()}) } } @@ -216,3 +239,27 @@ func extractAuthForm(s sessions.Session, form *model.OAuthAuthorize) error { s.Set(sessionState, uuid.NewString()) return s.Save() } + +func ensureUserIsAuthorizedOrRedirect(ctx *gin.Context, user *gtsmodel.User, account *gtsmodel.Account) bool { + if user.ConfirmedAt.IsZero() { + ctx.Redirect(http.StatusSeeOther, CheckYourEmailPath) + return false + } + + if !user.Approved { + ctx.Redirect(http.StatusSeeOther, WaitForApprovalPath) + return false + } + + if user.Disabled { + ctx.Redirect(http.StatusSeeOther, AccountDisabledPath) + return false + } + + if !account.SuspendedAt.IsZero() { + ctx.Redirect(http.StatusSeeOther, AccountDisabledPath) + return false + } + + return true +} diff --git a/internal/api/client/auth/authorize_test.go b/internal/api/client/auth/authorize_test.go new file mode 100644 index 000000000..8f16702da --- /dev/null +++ b/internal/api/client/auth/authorize_test.go @@ -0,0 +1,113 @@ +package auth_test + +import ( + "context" + "fmt" + "net/http" + "testing" + "time" + + "codeberg.org/gruf/go-errors" + "github.com/gin-contrib/sessions" + "github.com/stretchr/testify/suite" + "github.com/superseriousbusiness/gotosocial/internal/api/client/auth" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" +) + +type AuthAuthorizeTestSuite struct { + AuthStandardTestSuite +} + +type authorizeHandlerTestCase struct { + description string + mutateUserAccount func(*gtsmodel.User, *gtsmodel.Account) + expectedStatusCode int + expectedLocationHeader string +} + +func (suite *AuthAuthorizeTestSuite) TestAccountAuthorizeHandler() { + + var tests = []authorizeHandlerTestCase{ + { + description: "user has their email unconfirmed", + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + // nothing to do, weed_lord420 already has their email unconfirmed + }, + expectedStatusCode: http.StatusSeeOther, + expectedLocationHeader: auth.CheckYourEmailPath, + }, + { + description: "user has their email confirmed but is not approved", + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + user.ConfirmedAt = time.Now() + user.Email = user.UnconfirmedEmail + }, + expectedStatusCode: http.StatusSeeOther, + expectedLocationHeader: auth.WaitForApprovalPath, + }, + { + description: "user has their email confirmed and is approved, but User entity has been disabled", + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + user.ConfirmedAt = time.Now() + user.Email = user.UnconfirmedEmail + user.Approved = true + user.Disabled = true + }, + expectedStatusCode: http.StatusSeeOther, + expectedLocationHeader: auth.AccountDisabledPath, + }, + { + description: "user has their email confirmed and is approved, but Account entity has been suspended", + mutateUserAccount: func(user *gtsmodel.User, account *gtsmodel.Account) { + user.ConfirmedAt = time.Now() + user.Email = user.UnconfirmedEmail + user.Approved = true + user.Disabled = false + account.SuspendedAt = time.Now() + }, + expectedStatusCode: http.StatusSeeOther, + expectedLocationHeader: auth.AccountDisabledPath, + }, + } + + doTest := func(testCase authorizeHandlerTestCase) { + ctx, recorder := suite.newContext(http.MethodGet, auth.OauthAuthorizePath) + + user := suite.testUsers["unconfirmed_account"] + account := suite.testAccounts["unconfirmed_account"] + + testSession := sessions.Default(ctx) + testSession.Set(sessionUserID, user.ID) + testSession.Set(sessionClientID, suite.testApplications["application_1"].ClientID) + if err := testSession.Save(); err != nil { + panic(errors.WrapMsgf(err, "failed on case: %s", testCase.description)) + } + + testCase.mutateUserAccount(user, account) + + testCase.description = fmt.Sprintf("%s, %t, %s", user.Email, user.Disabled, account.SuspendedAt) + + user.UpdatedAt = time.Now() + err := suite.db.UpdateByPrimaryKey(context.Background(), user) + suite.NoError(err) + _, err = suite.db.UpdateAccount(context.Background(), account) + suite.NoError(err) + + // call the handler + suite.authModule.AuthorizeGETHandler(ctx) + + // 1. we should have a redirect + suite.Equal(testCase.expectedStatusCode, recorder.Code, fmt.Sprintf("failed on case: %s", testCase.description)) + + // 2. we should have a redirect to the check your email path, as this user has not confirmed their email yet. + suite.Equal(testCase.expectedLocationHeader, recorder.Header().Get("Location"), fmt.Sprintf("failed on case: %s", testCase.description)) + } + + for _, testCase := range tests { + doTest(testCase) + } +} + +func TestAccountUpdateTestSuite(t *testing.T) { + suite.Run(t, new(AuthAuthorizeTestSuite)) +} diff --git a/internal/api/security/tokencheck.go b/internal/api/security/tokencheck.go index b68f0b94f..e366af2ea 100644 --- a/internal/api/security/tokencheck.go +++ b/internal/api/security/tokencheck.go @@ -62,6 +62,22 @@ func (m *Module) TokenCheck(c *gin.Context) { l.Warnf("no user found for userID %s", userID) return } + + if user.ConfirmedAt.IsZero() { + l.Warnf("authenticated user %s has never confirmed thier email address", userID) + return + } + + if !user.Approved { + l.Warnf("authenticated user %s's account was never approved by an admin", userID) + return + } + + if user.Disabled { + l.Warnf("authenticated user %s's account was disabled'", userID) + return + } + c.Set(oauth.SessionAuthorizedUser, user) // fetch account for this token @@ -74,6 +90,12 @@ func (m *Module) TokenCheck(c *gin.Context) { l.Warnf("no account found for userID %s", userID) return } + + if !acct.SuspendedAt.IsZero() { + l.Warnf("authenticated user %s's account (accountId=%s) has been suspended", userID, user.AccountID) + return + } + c.Set(oauth.SessionAuthorizedAccount, acct) } diff --git a/internal/db/bundb/errors.go b/internal/db/bundb/errors.go index 7602d5e1d..d01720731 100644 --- a/internal/db/bundb/errors.go +++ b/internal/db/bundb/errors.go @@ -19,7 +19,7 @@ func processPostgresError(err error) db.Error { // (https://www.postgresql.org/docs/10/errcodes-appendix.html) switch pgErr.Code { case "23505" /* unique_violation */ : - return db.ErrAlreadyExists + return db.NewErrAlreadyExists(pgErr.Message) default: return err } @@ -36,7 +36,7 @@ func processSQLiteError(err error) db.Error { // Handle supplied error code: switch sqliteErr.Code() { case sqlite3.SQLITE_CONSTRAINT_UNIQUE: - return db.ErrAlreadyExists + return db.NewErrAlreadyExists(err.Error()) default: return err } diff --git a/internal/db/error.go b/internal/db/error.go index 984f96401..9ac0b6aa0 100644 --- a/internal/db/error.go +++ b/internal/db/error.go @@ -28,8 +28,19 @@ var ( ErrNoEntries Error = fmt.Errorf("no entries") // ErrMultipleEntries is returned when a caller expected ONE entry for a query, but multiples were found. ErrMultipleEntries Error = fmt.Errorf("multiple entries") - // ErrAlreadyExists is returned when a caller tries to insert a database entry that already exists in the db. - ErrAlreadyExists Error = fmt.Errorf("already exists") // ErrUnknown denotes an unknown database error. ErrUnknown Error = fmt.Errorf("unknown error") ) + +// ErrAlreadyExists is returned when a caller tries to insert a database entry that already exists in the db. +type ErrAlreadyExists struct { + message string +} + +func (e *ErrAlreadyExists) Error() string { + return e.message +} + +func NewErrAlreadyExists(msg string) error { + return &ErrAlreadyExists{message: msg} +} diff --git a/internal/federation/dereferencing/attachment.go b/internal/federation/dereferencing/attachment.go index 0c7005e23..36ff2734c 100644 --- a/internal/federation/dereferencing/attachment.go +++ b/internal/federation/dereferencing/attachment.go @@ -20,6 +20,7 @@ package dereferencing import ( "context" + "errors" "fmt" "net/url" @@ -60,7 +61,8 @@ func (d *deref) GetRemoteAttachment(ctx context.Context, requestingUsername stri } if err := d.db.Put(ctx, a); err != nil { - if err != db.ErrAlreadyExists { + var alreadyExistsError *db.ErrAlreadyExists + if !errors.As(err, &alreadyExistsError) { return nil, fmt.Errorf("GetRemoteAttachment: error inserting attachment: %s", err) } } diff --git a/internal/federation/federatingdb/create.go b/internal/federation/federatingdb/create.go index b07f4f04f..6c86151f3 100644 --- a/internal/federation/federatingdb/create.go +++ b/internal/federation/federatingdb/create.go @@ -231,7 +231,8 @@ func (f *federatingDB) createNote(ctx context.Context, note vocab.ActivityStream status.ID = statusID if err := f.db.PutStatus(ctx, status); err != nil { - if err == db.ErrAlreadyExists { + var alreadyExistsError *db.ErrAlreadyExists + if errors.As(err, &alreadyExistsError) { // the status already exists in the database, which means we've already handled everything else, // so we can just return nil here and be done with it. return nil diff --git a/internal/oauth/util.go b/internal/oauth/util.go index 540045f80..6f69f0ee4 100644 --- a/internal/oauth/util.go +++ b/internal/oauth/util.go @@ -78,25 +78,12 @@ func Authed(c *gin.Context, requireToken bool, requireApp bool, requireUser bool return nil, errors.New("application not supplied") } - if requireUser { - if a.User == nil { - return nil, errors.New("user not supplied") - } - if a.User.Disabled || !a.User.Approved { - return nil, errors.New("user disabled or not approved") - } - if a.User.Email == "" { - return nil, errors.New("user has no confirmed email address") - } + if requireUser && a.User == nil { + return nil, errors.New("user not supplied or not authorized") } - if requireAccount { - if a.Account == nil { - return nil, errors.New("account not supplied") - } - if !a.Account.SuspendedAt.IsZero() { - return nil, errors.New("account suspended") - } + if requireAccount && a.Account == nil { + return nil, errors.New("account not supplied or not authorized") } return a, nil diff --git a/internal/processing/status/util.go b/internal/processing/status/util.go index 05a3bf48e..f2640929d 100644 --- a/internal/processing/status/util.go +++ b/internal/processing/status/util.go @@ -223,8 +223,11 @@ func (p *processor) ProcessTags(ctx context.Context, form *apimodel.AdvancedStat return fmt.Errorf("error generating hashtags from status: %s", err) } for _, tag := range gtsTags { - if err := p.db.Put(ctx, tag); err != nil && err != db.ErrAlreadyExists { - return fmt.Errorf("error putting tags in db: %s", err) + if err := p.db.Put(ctx, tag); err != nil { + var alreadyExistsError *db.ErrAlreadyExists + if !errors.As(err, &alreadyExistsError) { + return fmt.Errorf("error putting tags in db: %s", err) + } } tags = append(tags, tag.ID) } diff --git a/internal/router/router.go b/internal/router/router.go index 88d900a9e..f1247d274 100644 --- a/internal/router/router.go +++ b/internal/router/router.go @@ -138,7 +138,7 @@ func New(ctx context.Context, db db.DB) (Router, error) { } // set template functions - loadTemplateFunctions(engine) + LoadTemplateFunctions(engine) // load templates onto the engine if err := loadTemplates(engine); err != nil { diff --git a/internal/router/session.go b/internal/router/session.go index 2127d70a7..066024601 100644 --- a/internal/router/session.go +++ b/internal/router/session.go @@ -33,8 +33,8 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/db" ) -// sessionOptions returns the standard set of options to use for each session. -func sessionOptions() sessions.Options { +// SessionOptions returns the standard set of options to use for each session. +func SessionOptions() sessions.Options { return sessions.Options{ Path: "/", Domain: viper.GetString(config.Keys.Host), @@ -75,7 +75,7 @@ func useSession(ctx context.Context, sessionDB db.Session, engine *gin.Engine) e } store := memstore.NewStore(rs.Auth, rs.Crypt) - store.Options(sessionOptions()) + store.Options(SessionOptions()) sessionName, err := SessionName() if err != nil { diff --git a/internal/router/template.go b/internal/router/template.go index 4cc9fde1d..1a0186d6d 100644 --- a/internal/router/template.go +++ b/internal/router/template.go @@ -31,7 +31,7 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/config" ) -// loadTemplates loads html templates for use by the given engine +// LoadTemplates loads html templates for use by the given engine func loadTemplates(engine *gin.Engine) error { cwd, err := os.Getwd() if err != nil { @@ -39,8 +39,13 @@ func loadTemplates(engine *gin.Engine) error { } templateBaseDir := viper.GetString(config.Keys.WebTemplateBaseDir) - tmPath := filepath.Join(cwd, fmt.Sprintf("%s*", templateBaseDir)) + _, err = os.Stat(filepath.Join(cwd, templateBaseDir, "index.tmpl")) + if err != nil { + return fmt.Errorf("%s doesn't seem to contain the templates; index.tmpl is missing: %s", filepath.Join(cwd, templateBaseDir), err) + } + + tmPath := filepath.Join(cwd, fmt.Sprintf("%s*", templateBaseDir)) engine.LoadHTMLGlob(tmPath) return nil } @@ -87,7 +92,7 @@ func visibilityIcon(visibility model.Visibility) template.HTML { return template.HTML(fmt.Sprintf(``, icon.label, icon.faIcon)) } -func loadTemplateFunctions(engine *gin.Engine) { +func LoadTemplateFunctions(engine *gin.Engine) { engine.SetFuncMap(template.FuncMap{ "noescape": noescape, "oddOrEven": oddOrEven, diff --git a/testrig/router.go b/testrig/router.go index 6842c0210..7f883cd6a 100644 --- a/testrig/router.go +++ b/testrig/router.go @@ -20,7 +20,14 @@ package testrig import ( "context" + "fmt" + "os" + "path/filepath" + "runtime" + "github.com/gin-gonic/gin" + "github.com/spf13/viper" + "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/router" ) @@ -33,3 +40,26 @@ func NewTestRouter(db db.DB) router.Router { } return r } + +// ConfigureTemplatesWithGin will panic on any errors related to template loading during tests +func ConfigureTemplatesWithGin(engine *gin.Engine) { + + router.LoadTemplateFunctions(engine) + + // https://stackoverflow.com/questions/31873396/is-it-possible-to-get-the-current-root-of-package-structure-as-a-string-in-golan + _, runtimeCallerLocation, _, _ := runtime.Caller(0) + projectRoot, err := filepath.Abs(filepath.Join(filepath.Dir(runtimeCallerLocation), "../")) + if err != nil { + panic(err) + } + + templateBaseDir := viper.GetString(config.Keys.WebTemplateBaseDir) + + _, err = os.Stat(filepath.Join(projectRoot, templateBaseDir, "index.tmpl")) + if err != nil { + panic(fmt.Errorf("%s doesn't seem to contain the templates; index.tmpl is missing: %s", filepath.Join(projectRoot, templateBaseDir), err)) + } + + tmPath := filepath.Join(projectRoot, fmt.Sprintf("%s*", templateBaseDir)) + engine.LoadHTMLGlob(tmPath) +} diff --git a/web/assets/base.css b/web/assets/base.css index b28cf2533..e105707c2 100644 --- a/web/assets/base.css +++ b/web/assets/base.css @@ -165,6 +165,25 @@ section.login form button { grid-column: 2; } +section.error { + display: flex; + flex-direction: row; + align-items: center; +} +section.error span { + font-size: 2em; +} +section.error pre { + border: 1px solid #ff000080; + margin-left: 1em; + padding: 0 0.7em; + border-radius: 0.5em; + background-color: #ff000010; + font-size: 1.3em; + white-space: pre-wrap; +} + + input, select, textarea { border: 1px solid #fafaff; color: #fafaff; diff --git a/web/gotosocial-styling/templates/base.css b/web/gotosocial-styling/templates/base.css index 8b2f14fbe..4ac639a38 100644 --- a/web/gotosocial-styling/templates/base.css +++ b/web/gotosocial-styling/templates/base.css @@ -165,6 +165,24 @@ section.login { } } +section.error { + display: flex; + flex-direction: row; + align-items: center; + span { + font-size: 2em; + } + pre { + border: 1px solid #ff000080; + margin-left: 1em; + padding: 0 0.7em; + border-radius: 0.5em; + background-color: #ff000010; + font-size: 1.3em; + white-space: pre-wrap; + } +} + input, select, textarea { border: 1px solid $fg; color: $fg; diff --git a/web/template/authorize.tmpl b/web/template/authorize.tmpl index b6eef9561..68a6a2f29 100644 --- a/web/template/authorize.tmpl +++ b/web/template/authorize.tmpl @@ -2,7 +2,13 @@

Hi {{.user}}!

-

Application {{.appname}} {{if len .appwebsite | eq 0 | not}}({{.appwebsite}}) {{end}}would like to perform actions on your behalf, with scope {{.scope}}.

+

+ Application {{.appname}} + {{if len .appwebsite | eq 0 | not}} + ({{.appwebsite}}) + {{end}} + would like to perform actions on your behalf, with scope {{.scope}}. +

The application will redirect to {{.redirect}} to continue.

+{{ template "footer.tmpl" .}} \ No newline at end of file -- cgit v1.3