Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redirect user to webapp one upon successful signup #1284

Merged
merged 1 commit into from Jul 18, 2023

Conversation

KinyaElGrande
Copy link
Contributor

@KinyaElGrande KinyaElGrande commented Jun 28, 2023

Ref: #1213

This PR introduces a change that is meant to enhance the signup flow by introducing the following changes:

  1. After successful signup or email confirmation, the user is automatically redirected to the webapp one.
  2. In case the authorization client is not trusted, the user will be directed to the authorize-client page.
  3. If auth client is nil, the user will be redirected to their own profile page.

@KinyaElGrande KinyaElGrande linked an issue Jun 28, 2023 that may be closed by this pull request
Copy link
Member

@tjerman tjerman left a comment

Choose a reason for hiding this comment

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

lgtm

@katrinDY
Copy link
Contributor

If I disable the trusted checkbox in admin -> create a new user in a private tab -> press deny -> logout -> try to create a new user/login -> I get err status 500

goroutine 1480 [running]:
runtime/debug.Stack()
        runtime/debug/stack.go:24 +0x65
runtime/debug.PrintStack()
        runtime/debug/stack.go:16 +0x19
github.com/cortezaproject/corteza/server/pkg/api/server.panicRecovery({0x6528570, 0xc001d01100}, {0x6525db0, 0xc00071a540})
        github.com/cortezaproject/corteza/server/pkg/api/server/middleware.go:43 +0x111
panic({0x5b67b80, 0x7725400})
        runtime/panic.go:884 +0x212
github.com/getsentry/sentry-go/http.(*Handler).recoverWithSentry(0xc000ece3e0, 0x0?, 0xc001073700)
        github.com/getsentry/sentry-go@v0.13.0/http/sentryhttp.go:117 +0xf0
panic({0x5b67b80, 0x7725400})
        runtime/panic.go:884 +0x212
github.com/cortezaproject/corteza/server/auth/handlers.(*AuthHandlers).signupProc(0xc001201a00, 0xc000c2fc20)
        github.com/cortezaproject/corteza/server/auth/handlers/handle_signup.go:49 +0x73e
github.com/cortezaproject/corteza/server/auth/handlers.anonyOnly.func1(0xc002b098f0?)
        github.com/cortezaproject/corteza/server/auth/handlers/handler.go:470 +0xc2
github.com/cortezaproject/corteza/server/auth/handlers.(*AuthHandlers).onlyIfSignupEnabled.func1(0xc000ba48d0?)
        github.com/cortezaproject/corteza/server/auth/handlers/handle_signup.go:167 +0x33
github.com/cortezaproject/corteza/server/auth/handlers.(*AuthHandlers).handle.func1.1(0xc00019f800, 0xc000c2fc20, 0xc001312b88, 0xc001201a00, {0x6525db0, 0xc00071a540})
        github.com/cortezaproject/corteza/server/auth/handlers/handler.go:235 +0x742
github.com/cortezaproject/corteza/server/auth/handlers.(*AuthHandlers).handle.func1({0x6525db0?, 0xc00071a540}, 0xc00019f800)
        github.com/cortezaproject/corteza/server/auth/handlers/handler.go:267 +0x415
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0xc000984ec0?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/auth.ExtraReqInfoMiddleware.func1({0x6525db0, 0xc00071a540}, 0xc00019e100)
        github.com/cortezaproject/corteza/server/pkg/auth/extra.go:17 +0x20d
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x65072f0?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/auth/handlers.(*AuthHandlers).MountHttpRoutes.func1.1.1({0x6525db0, 0xc00071a540}, 0xc001289f00)
        github.com/cortezaproject/corteza/server/auth/handlers/routes.go:31 +0x16e
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x77a9ce8?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/locale.DetectLanguage.func1.1({0x6525db0, 0xc00071a540}, 0xc001461b00)
        github.com/cortezaproject/corteza/server/pkg/locale/http.go:26 +0x2ad
net/http.HandlerFunc.ServeHTTP(0xc001339dd0?, {0x6525db0?, 0xc00071a540?}, 0xc00041e240?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/chi/v5.(*ChainHandler).ServeHTTP(0x5b4dda0?, {0x6525db0?, 0xc00071a540?}, 0xc0012d1420?)
        github.com/go-chi/chi/v5@v5.0.7/chain.go:31 +0x2c
github.com/go-chi/chi/v5.(*Mux).routeHTTP(0xc0006540c0, {0x6525db0, 0xc00071a540}, 0xc001461b00)
        github.com/go-chi/chi/v5@v5.0.7/mux.go:442 +0x216
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x77a9ce8?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/auth.verifier.func1.1({0x6525db0, 0xc00071a540}, 0xc001461a00)
        github.com/cortezaproject/corteza/server/pkg/auth/token_middleware.go:42 +0x291
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x77a9ce8?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/api/server.contextLogger.func1.1({0x6525db0, 0xc00071a540}, 0xc001461900)
        github.com/cortezaproject/corteza/server/pkg/api/server/logger.go:28 +0x394
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x77a9ce8?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/api.DebugToContext.func1.1({0x6525db0, 0xc00071a540}, 0xc001461800)
        github.com/cortezaproject/corteza/server/pkg/api/debug.go:15 +0x151
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x65072f0?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/chi/v5/middleware.RequestID.func1({0x6525db0, 0xc00071a540}, 0xc001461700)
        github.com/go-chi/chi/v5@v5.0.7/middleware/request_id.go:76 +0x22d
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x65072f0?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/api.RemoteAddrToContext.func1({0x6525db0, 0xc00071a540}, 0xc001461600)
        github.com/cortezaproject/corteza/server/pkg/api/ipaddr.go:17 +0x16e
net/http.HandlerFunc.ServeHTTP(0xc001461600?, {0x6525db0?, 0xc00071a540?}, 0xc001461600?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/chi/v5/middleware.RealIP.func1({0x6525db0, 0xc00071a540}, 0xc001461600)
        github.com/go-chi/chi/v5@v5.0.7/middleware/realip.go:35 +0x9e
net/http.HandlerFunc.ServeHTTP(0x6528618?, {0x6525db0?, 0xc00071a540?}, 0x77a9ce8?)
        net/http/server.go:2109 +0x2f
github.com/cortezaproject/corteza/server/pkg/locale.DetectLanguage.func1.1({0x6525db0, 0xc00071a540}, 0xc001073700)
        github.com/cortezaproject/corteza/server/pkg/locale/http.go:26 +0x2ad
net/http.HandlerFunc.ServeHTTP(0xc001333860?, {0x6525db0?, 0xc00071a540?}, 0xc001073700?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/cors.(*Cors).Handler.func1({0x6525db0, 0xc00071a540}, 0xc001073700)
        github.com/go-chi/cors@v1.2.1/cors.go:228 +0x1bd
net/http.HandlerFunc.ServeHTTP(0xc00122e0f0?, {0x6525db0?, 0xc00071a540?}, 0xb?)
        net/http/server.go:2109 +0x2f
github.com/getsentry/sentry-go/http.(*Handler).handle.func1({0x6525db0, 0xc00071a540}, 0xc001073600)
        github.com/getsentry/sentry-go@v0.13.0/http/sentryhttp.go:103 +0x3c3
net/http.HandlerFunc.ServeHTTP(0xc00104ec30?, {0x6525db0?, 0xc00071a540?}, 0xc000ba5800?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/chi/v5.(*Mux).ServeHTTP(0xc0006540c0, {0x6525db0, 0xc00071a540}, 0xc001073600)
        github.com/go-chi/chi/v5@v5.0.7/mux.go:71 +0x355
github.com/go-chi/chi/v5.(*Mux).Mount.func1({0x6525db0, 0xc00071a540}, 0xc001073600)
        github.com/go-chi/chi/v5@v5.0.7/mux.go:314 +0x19c
net/http.HandlerFunc.ServeHTTP(0x5b4dda0?, {0x6525db0?, 0xc00071a540?}, 0xc0003cce65?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/chi/v5.(*Mux).routeHTTP(0xc000654060, {0x6525db0, 0xc00071a540}, 0xc001073600)
        github.com/go-chi/chi/v5@v5.0.7/mux.go:442 +0x216
net/http.HandlerFunc.ServeHTTP(0xc00140ef00?, {0x6525db0?, 0xc00071a540?}, 0xc001073600?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/cors.(*Cors).Handler.func1({0x6525db0, 0xc00071a540}, 0xc001073600)
        github.com/go-chi/cors@v1.2.1/cors.go:228 +0x1bd
net/http.HandlerFunc.ServeHTTP(0x6528570?, {0x6525db0?, 0xc00071a540?}, 0x7724de0?)
        net/http/server.go:2109 +0x2f
github.com/go-chi/chi/v5.(*Mux).ServeHTTP(0xc000654060, {0x6525db0, 0xc00071a540}, 0xc001073500)
        github.com/go-chi/chi/v5@v5.0.7/mux.go:88 +0x310
github.com/cortezaproject/corteza/server/pkg/api/server.(*demux).ServeHTTP(0x0?, {0x6525db0?, 0xc00071a540}, 0xc001073500)
        github.com/cortezaproject/corteza/server/pkg/api/server/demux.go:51 +0x113
net/http.serverHandler.ServeHTTP({0x651f3f0?}, {0x6525db0, 0xc00071a540}, 0xc001073500)
        net/http/server.go:2947 +0x30c
net/http.(*conn).serve(0xc002b5e0a0, {0x6528618, 0xc0002aea20})
        net/http/server.go:1991 +0x607
created by net/http.(*Server).Serve
        net/http/server.go:3102 +0x4db

@katrinDY
Copy link
Contributor

katrinDY commented Jul 4, 2023

LGTM except for one small thing - this message:
Screenshot 2023-07-05 at 14 30 47

Maybe capitalize the first letter of the message, remove the comma before more permissions and make the message - Cannot authorize Corteza Web Applications. No permissions or No permissions to authorize Corteza Web Applications. Please, discuss with Jože

@KinyaElGrande KinyaElGrande force-pushed the 2023.3.x-fix-sign-up-flow branch 2 times, most recently from 8a25f79 to 0e8895a Compare July 7, 2023 12:14
@KinyaElGrande
Copy link
Contributor Author

We opted for Failed to authorize {{auth client }}. No permissions alert message when the auth client authorization process fails.

Copy link
Member

@tjerman tjerman left a comment

Choose a reason for hiding this comment

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

Few finishing touches; else lgtm 👍
Good job!

@@ -11,4 +11,4 @@ template:
errors:
invalid-user: Cannot continue with unauthorized email, visit <a data-test-id="link-redirect-to-profile" href="{{link}}">your profile</a> and resolve the issue.
alerts:
denied: cannot authorize {{client}}, no permissions
denied: Failed to authorize {{client}}. No permissions
Copy link
Member

Choose a reason for hiding this comment

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

Think we should go more in the lines of

Suggested change
denied: Failed to authorize {{client}}. No permissions
failed to authorize {{client}}: insufficient permissions

server/app/boot_auth.go Show resolved Hide resolved
@katrinDY
Copy link
Contributor

Two things:

  • the page for authorizing a client is always shown if the auth client isn't trusted. I visited admin 3 times and had to confirm that I'd like for Corteza to perform actions on my behalf. In the previous version of the code I only had to confirm once per webapp
  • The first letter of the warning msg isn't capitalized. Not sure if it's intended
Screenshot 2023-07-11 at 12 38 09

@katrinDY
Copy link
Contributor

lgtm

If the authorize client isn't trusted, the user is taken to authorize-client page
@KinyaElGrande KinyaElGrande merged commit 9615a91 into 2023.3.x Jul 18, 2023
1 check passed
@KinyaElGrande KinyaElGrande deleted the 2023.3.x-fix-sign-up-flow branch July 18, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improvement of Corteza signup flow
3 participants