From 4325e0fb3d144f9bbfeabe229237953119f5ca19 Mon Sep 17 00:00:00 2001 From: Henry Date: Wed, 24 Mar 2021 18:31:37 +0100 Subject: [PATCH] various fixes * fill in sid and sc * fix logout * cascade member removal * fix links * trim suffix from signature * fix /sse/login link * fix sse links and filenames * fix logout * fix typos * fix test compilation * fix bridge test * correct alias url test * add some comments * fix potentiall "can't send" deadlock on bridge some documentation and license headers --- cmd/server/main.go | 3 +- internal/signinwithssb/bridge.go | 39 +++++++--- internal/signinwithssb/bridge_test.go | 12 ++- internal/signinwithssb/challenges.go | 2 + internal/signinwithssb/simple_test.go | 2 + muxrpc/handlers/signinwithssb/withssb.go | 22 +++++- muxrpc/test/go/alias_test.go | 7 +- muxrpc/test/go/utils_test.go | 4 +- muxrpc/test/nodejs/setup_test.go | 8 +- roomdb/interface.go | 3 + roomdb/mockdb/auth.go | 76 +++++++++++++++++++ roomdb/sqlite/auth_withssb.go | 6 ++ roomdb/sqlite/migrations/01-consolidated.sql | 6 +- roomdb/sqlite/migrations/03-siwssb-tokens.sql | 4 +- .../{events-demo.js => login-events.js} | 4 +- web/handlers/aliases.go | 2 +- web/handlers/auth/handler.go | 2 - web/handlers/auth/withssb.go | 75 +++++++++--------- web/handlers/auth_test.go | 24 +++--- web/handlers/http.go | 9 +++ web/handlers/setup_test.go | 8 +- web/i18n/defaults/active.en.toml | 3 +- web/members/helper.go | 2 + web/members/testing.go | 2 + web/router/auth.go | 14 ++-- web/router/complete.go | 4 +- .../admin/members-remove-confirm.tmpl | 4 +- web/templates/auth/withssb_server_start.tmpl | 9 +-- web/templates/auth/withssb_sign_in.tmpl | 5 +- 29 files changed, 260 insertions(+), 101 deletions(-) rename web/assets/{events-demo.js => login-events.js} (83%) diff --git a/cmd/server/main.go b/cmd/server/main.go index 1603b7c..17b3c89 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -19,8 +19,6 @@ import ( "syscall" "time" - "github.com/ssb-ngi-pointer/go-ssb-room/internal/signinwithssb" - // debug "net/http" _ "net/http/pprof" @@ -32,6 +30,7 @@ import ( "go.cryptoscope.co/muxrpc/v2/debug" "github.com/ssb-ngi-pointer/go-ssb-room/internal/repo" + "github.com/ssb-ngi-pointer/go-ssb-room/internal/signinwithssb" "github.com/ssb-ngi-pointer/go-ssb-room/roomdb/sqlite" "github.com/ssb-ngi-pointer/go-ssb-room/roomsrv" mksrv "github.com/ssb-ngi-pointer/go-ssb-room/roomsrv" diff --git a/internal/signinwithssb/bridge.go b/internal/signinwithssb/bridge.go index dca602a..1460f18 100644 --- a/internal/signinwithssb/bridge.go +++ b/internal/signinwithssb/bridge.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package signinwithssb import ( @@ -29,8 +31,7 @@ func NewSignalBridge() *SignalBridge { } // RegisterSession registers a new session on the bridge. -// It returns a channel from which future events can be read -// and the server challenge, which acts as the session key. +// It returns a fresh server challenge, which acts as the session key. func (sb *SignalBridge) RegisterSession() string { sb.mu.Lock() defer sb.mu.Unlock() @@ -38,7 +39,7 @@ func (sb *SignalBridge) RegisterSession() string { c := GenerateChallenge() _, used := sb.sessions[c] if used { - for used { // generate new challanges until we have an un-used one + for used { // generate new challenges until we have an un-used one c = GenerateChallenge() _, used = sb.sessions[c] } @@ -57,6 +58,8 @@ func (sb *SignalBridge) RegisterSession() string { return c } +// GetEventChannel returns the channel for the passed challenge from which future events can be read. +// If sc doesn't exist, the 2nd argument is false. func (sb *SignalBridge) GetEventChannel(sc string) (<-chan Event, bool) { sb.mu.Lock() defer sb.mu.Unlock() @@ -64,7 +67,7 @@ func (sb *SignalBridge) GetEventChannel(sc string) (<-chan Event, bool) { return ch, has } -// CompleteSession uses the passed challange to send on and close the open channel. +// CompleteSession uses the passed challenge to send on and close the open channel. // It will return an error if the session doesn't exist. func (sb *SignalBridge) CompleteSession(sc string, success bool, token string) error { sb.mu.Lock() @@ -75,14 +78,28 @@ func (sb *SignalBridge) CompleteSession(sc string, success bool, token string) e return fmt.Errorf("no such session") } - ch <- Event{ - Worked: success, - Token: token, - } - close(ch) + var ( + err error + timeout = time.NewTimer(1 * time.Minute) - // remove session + evt = Event{ + Worked: success, + Token: token, + } + ) + + // handle what happens if the sse client isn't connected + select { + case <-timeout.C: + err = fmt.Errorf("faled to send completed session") + + case ch <- evt: + timeout.Stop() + } + + // session is finalized either way + close(ch) delete(sb.sessions, sc) - return nil + return err } diff --git a/internal/signinwithssb/bridge_test.go b/internal/signinwithssb/bridge_test.go index a72a8d1..53554a0 100644 --- a/internal/signinwithssb/bridge_test.go +++ b/internal/signinwithssb/bridge_test.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package signinwithssb import ( @@ -13,18 +15,21 @@ func TestBridge(t *testing.T) { sb := NewSignalBridge() // try to use a non-existant session - err := sb.CompleteSession("nope", false) + err := sb.CompleteSession("nope", false, "nope-token") a.Error(err) // make a new session - updates, sc := sb.RegisterSession() + sc := sb.RegisterSession() b, err := DecodeChallengeString(sc) a.NoError(err) a.Len(b, challengeLength) + updates, has := sb.GetEventChannel(sc) + a.True(has) + go func() { - err := sb.CompleteSession(sc, true) + err := sb.CompleteSession(sc, true, "a token") a.NoError(err) }() @@ -33,6 +38,7 @@ func TestBridge(t *testing.T) { select { case evt := <-updates: a.True(evt.Worked) + a.Equal("a token", evt.Token) default: t.Error("no updates") } diff --git a/internal/signinwithssb/challenges.go b/internal/signinwithssb/challenges.go index 559cd81..7514376 100644 --- a/internal/signinwithssb/challenges.go +++ b/internal/signinwithssb/challenges.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package signinwithssb import ( diff --git a/internal/signinwithssb/simple_test.go b/internal/signinwithssb/simple_test.go index 6dca144..064735f 100644 --- a/internal/signinwithssb/simple_test.go +++ b/internal/signinwithssb/simple_test.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package signinwithssb import ( diff --git a/muxrpc/handlers/signinwithssb/withssb.go b/muxrpc/handlers/signinwithssb/withssb.go index 1a0e9f8..7726678 100644 --- a/muxrpc/handlers/signinwithssb/withssb.go +++ b/muxrpc/handlers/signinwithssb/withssb.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package signinwithssb import ( @@ -5,6 +7,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "strings" kitlog "github.com/go-kit/kit/log" "go.cryptoscope.co/muxrpc/v2" @@ -16,7 +19,7 @@ import ( refs "go.mindeco.de/ssb-refs" ) -// Handler implements the muxrpc methods for alias registration and recvocation +// Handler implements the muxrpc methods for the "Sign-in with SSB" calls. SendSolution and InvalidateAllSolutions. type Handler struct { logger kitlog.Logger self refs.FeedRef @@ -29,7 +32,7 @@ type Handler struct { roomDomain string // the http(s) domain of the room to signal redirect addresses } -// New returns a fresh alias muxrpc handler +// New returns the muxrpc handler for Sign-in with SSB func New( log kitlog.Logger, self refs.FeedRef, @@ -50,6 +53,9 @@ func New( return h } +// SendSolution implements the receiving end of httpAuth.sendSolution. +// It recevies three parameters [sc, cc, sol], does the validation and if it passes creates a token +// and signals the created token to the SSE HTTP handler using the signal bridge. func (h Handler) SendSolution(ctx context.Context, req *muxrpc.Request) (interface{}, error) { clientID, err := network.GetFeedRefFromAddr(req.RemoteAddr()) if err != nil { @@ -76,7 +82,7 @@ func (h Handler) SendSolution(ctx context.Context, req *muxrpc.Request) (interfa sol.ClientID = *clientID sol.ClientChallenge = params[1] - sig, err := base64.StdEncoding.DecodeString(params[2]) + sig, err := base64.StdEncoding.DecodeString(strings.TrimSuffix(params[2], ".sig.ed25519")) if err != nil { h.bridge.CompleteSession(sol.ServerChallenge, false, "") return nil, fmt.Errorf("signature is not valid base64 data: %w", err) @@ -92,10 +98,16 @@ func (h Handler) SendSolution(ctx context.Context, req *muxrpc.Request) (interfa return nil, err } - h.bridge.CompleteSession(sol.ServerChallenge, true, tok) + err = h.bridge.CompleteSession(sol.ServerChallenge, true, tok) + if err != nil { + h.sessions.RemoveToken(ctx, tok) + return nil, err + } + return true, nil } +// InvalidateAllSolutions implements the muxrpc call httpAuth.invalidateAllSolutions func (h Handler) InvalidateAllSolutions(ctx context.Context, req *muxrpc.Request) (interface{}, error) { // get the feed from the muxrpc connection clientID, err := network.GetFeedRefFromAddr(req.RemoteAddr()) @@ -103,11 +115,13 @@ func (h Handler) InvalidateAllSolutions(ctx context.Context, req *muxrpc.Request return nil, err } + // lookup the member member, err := h.members.GetByFeed(ctx, *clientID) if err != nil { return nil, err } + // delete all SIWSSB sessions of that member err = h.sessions.WipeTokensForMember(ctx, member.ID) if err != nil { return nil, err diff --git a/muxrpc/test/go/alias_test.go b/muxrpc/test/go/alias_test.go index 368f5b9..b871627 100644 --- a/muxrpc/test/go/alias_test.go +++ b/muxrpc/test/go/alias_test.go @@ -10,6 +10,8 @@ import ( "testing" "time" + "github.com/ssb-ngi-pointer/go-ssb-room/web/router" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.cryptoscope.co/muxrpc/v2" @@ -106,7 +108,10 @@ func TestAliasRegister(t *testing.T) { r.NoError(err) t.Log("got URL:", resolveURL) a.Equal("srv", resolveURL.Host) - a.Equal("/bob", resolveURL.Path) + + wantURL, err := router.CompleteApp().Get(router.CompleteAliasResolve).URL("alias", "bob") + r.NoError(err) + a.Equal(wantURL.Path, resolveURL.Path) // server should have the alias now alias, err := serv.Aliases.Resolve(ctx, "bob") diff --git a/muxrpc/test/go/utils_test.go b/muxrpc/test/go/utils_test.go index c988b4b..aedd3d4 100644 --- a/muxrpc/test/go/utils_test.go +++ b/muxrpc/test/go/utils_test.go @@ -23,6 +23,7 @@ import ( "github.com/ssb-ngi-pointer/go-ssb-room/internal/maybemod/testutils" "github.com/ssb-ngi-pointer/go-ssb-room/internal/network" "github.com/ssb-ngi-pointer/go-ssb-room/internal/repo" + "github.com/ssb-ngi-pointer/go-ssb-room/internal/signinwithssb" "github.com/ssb-ngi-pointer/go-ssb-room/roomdb/sqlite" "github.com/ssb-ngi-pointer/go-ssb-room/roomsrv" ) @@ -88,7 +89,8 @@ func makeNamedTestBot(t testing.TB, name string, opts []roomsrv.Option) (roomdb. t.Log("db close failed: ", err) } }) - theBot, err := roomsrv.New(db.Members, db.Aliases, name, botOptions...) + sb := signinwithssb.NewSignalBridge() + theBot, err := roomsrv.New(db.Members, db.Aliases, db.AuthWithSSB, sb, name, botOptions...) r.NoError(err) return db.Members, theBot } diff --git a/muxrpc/test/nodejs/setup_test.go b/muxrpc/test/nodejs/setup_test.go index d769a7b..026af33 100644 --- a/muxrpc/test/nodejs/setup_test.go +++ b/muxrpc/test/nodejs/setup_test.go @@ -28,7 +28,9 @@ import ( "github.com/ssb-ngi-pointer/go-ssb-room/internal/maybemod/testutils" "github.com/ssb-ngi-pointer/go-ssb-room/internal/network" + "github.com/ssb-ngi-pointer/go-ssb-room/internal/signinwithssb" "github.com/ssb-ngi-pointer/go-ssb-room/roomdb" + "github.com/ssb-ngi-pointer/go-ssb-room/roomdb/mockdb" "github.com/ssb-ngi-pointer/go-ssb-room/roomsrv" refs "go.mindeco.de/ssb-refs" ) @@ -117,7 +119,11 @@ func (ts *testSession) startGoServer( }), ) - srv, err := roomsrv.New(membersDB, aliasDB, "go.test.room.server", opts...) + // not needed for testing yet + sb := signinwithssb.NewSignalBridge() + authSessionsDB := new(mockdb.FakeAuthWithSSBService) + + srv, err := roomsrv.New(membersDB, aliasDB, authSessionsDB, sb, "go.test.room.server", opts...) r.NoError(err, "failed to init tees a server") ts.t.Logf("go server: %s", srv.Whoami().Ref()) ts.t.Cleanup(func() { diff --git a/roomdb/interface.go b/roomdb/interface.go index f1d359b..daa71a1 100644 --- a/roomdb/interface.go +++ b/roomdb/interface.go @@ -44,6 +44,9 @@ type AuthWithSSBService interface { // CheckToken checks if the passed token is still valid and returns the member id if so CheckToken(ctx context.Context, token string) (int64, error) + // RemoveToken removes a single token from the database + RemoveToken(ctx context.Context, token string) error + // WipeTokensForMember deletes all tokens currently held for that member WipeTokensForMember(ctx context.Context, memberID int64) error } diff --git a/roomdb/mockdb/auth.go b/roomdb/mockdb/auth.go index a434f2d..a7a213c 100644 --- a/roomdb/mockdb/auth.go +++ b/roomdb/mockdb/auth.go @@ -37,6 +37,18 @@ type FakeAuthWithSSBService struct { result1 string result2 error } + RemoveTokenStub func(context.Context, string) error + removeTokenMutex sync.RWMutex + removeTokenArgsForCall []struct { + arg1 context.Context + arg2 string + } + removeTokenReturns struct { + result1 error + } + removeTokenReturnsOnCall map[int]struct { + result1 error + } WipeTokensForMemberStub func(context.Context, int64) error wipeTokensForMemberMutex sync.RWMutex wipeTokensForMemberArgsForCall []struct { @@ -183,6 +195,68 @@ func (fake *FakeAuthWithSSBService) CreateTokenReturnsOnCall(i int, result1 stri }{result1, result2} } +func (fake *FakeAuthWithSSBService) RemoveToken(arg1 context.Context, arg2 string) error { + fake.removeTokenMutex.Lock() + ret, specificReturn := fake.removeTokenReturnsOnCall[len(fake.removeTokenArgsForCall)] + fake.removeTokenArgsForCall = append(fake.removeTokenArgsForCall, struct { + arg1 context.Context + arg2 string + }{arg1, arg2}) + stub := fake.RemoveTokenStub + fakeReturns := fake.removeTokenReturns + fake.recordInvocation("RemoveToken", []interface{}{arg1, arg2}) + fake.removeTokenMutex.Unlock() + if stub != nil { + return stub(arg1, arg2) + } + if specificReturn { + return ret.result1 + } + return fakeReturns.result1 +} + +func (fake *FakeAuthWithSSBService) RemoveTokenCallCount() int { + fake.removeTokenMutex.RLock() + defer fake.removeTokenMutex.RUnlock() + return len(fake.removeTokenArgsForCall) +} + +func (fake *FakeAuthWithSSBService) RemoveTokenCalls(stub func(context.Context, string) error) { + fake.removeTokenMutex.Lock() + defer fake.removeTokenMutex.Unlock() + fake.RemoveTokenStub = stub +} + +func (fake *FakeAuthWithSSBService) RemoveTokenArgsForCall(i int) (context.Context, string) { + fake.removeTokenMutex.RLock() + defer fake.removeTokenMutex.RUnlock() + argsForCall := fake.removeTokenArgsForCall[i] + return argsForCall.arg1, argsForCall.arg2 +} + +func (fake *FakeAuthWithSSBService) RemoveTokenReturns(result1 error) { + fake.removeTokenMutex.Lock() + defer fake.removeTokenMutex.Unlock() + fake.RemoveTokenStub = nil + fake.removeTokenReturns = struct { + result1 error + }{result1} +} + +func (fake *FakeAuthWithSSBService) RemoveTokenReturnsOnCall(i int, result1 error) { + fake.removeTokenMutex.Lock() + defer fake.removeTokenMutex.Unlock() + fake.RemoveTokenStub = nil + if fake.removeTokenReturnsOnCall == nil { + fake.removeTokenReturnsOnCall = make(map[int]struct { + result1 error + }) + } + fake.removeTokenReturnsOnCall[i] = struct { + result1 error + }{result1} +} + func (fake *FakeAuthWithSSBService) WipeTokensForMember(arg1 context.Context, arg2 int64) error { fake.wipeTokensForMemberMutex.Lock() ret, specificReturn := fake.wipeTokensForMemberReturnsOnCall[len(fake.wipeTokensForMemberArgsForCall)] @@ -252,6 +326,8 @@ func (fake *FakeAuthWithSSBService) Invocations() map[string][][]interface{} { defer fake.checkTokenMutex.RUnlock() fake.createTokenMutex.RLock() defer fake.createTokenMutex.RUnlock() + fake.removeTokenMutex.RLock() + defer fake.removeTokenMutex.RUnlock() fake.wipeTokensForMemberMutex.RLock() defer fake.wipeTokensForMemberMutex.RUnlock() copiedInvocations := map[string][][]interface{}{} diff --git a/roomdb/sqlite/auth_withssb.go b/roomdb/sqlite/auth_withssb.go index ccd51a7..b24ee45 100644 --- a/roomdb/sqlite/auth_withssb.go +++ b/roomdb/sqlite/auth_withssb.go @@ -114,6 +114,12 @@ func (a AuthWithSSB) CheckToken(ctx context.Context, token string) (int64, error return memberID, nil } +// RemoveToken removes a single token from the database +func (a AuthWithSSB) RemoveToken(ctx context.Context, token string) error { + _, err := models.SIWSSBSessions(qm.Where("token = ?", token)).DeleteAll(ctx, a.db) + return err +} + // WipeTokensForMember deletes all tokens currently held for that member func (a AuthWithSSB) WipeTokensForMember(ctx context.Context, memberID int64) error { return transact(a.db, func(tx *sql.Tx) error { diff --git a/roomdb/sqlite/migrations/01-consolidated.sql b/roomdb/sqlite/migrations/01-consolidated.sql index 469e07a..baa8b1e 100644 --- a/roomdb/sqlite/migrations/01-consolidated.sql +++ b/roomdb/sqlite/migrations/01-consolidated.sql @@ -18,7 +18,7 @@ CREATE TABLE fallback_passwords ( member_id INTEGER NOT NULL, - FOREIGN KEY ( member_id ) REFERENCES members( "id" ) + FOREIGN KEY ( member_id ) REFERENCES members( "id" ) ON DELETE CASCADE ); CREATE INDEX fallback_passwords_by_login ON fallback_passwords(login); @@ -32,7 +32,7 @@ CREATE TABLE invites ( alias_suggestion TEXT NOT NULL DEFAULT "", -- optional active boolean NOT NULL DEFAULT TRUE, - FOREIGN KEY ( created_by ) REFERENCES members( "id" ) + FOREIGN KEY ( created_by ) REFERENCES members( "id" ) ON DELETE CASCADE ); CREATE INDEX invite_active_ids ON invites(id) WHERE active=TRUE; CREATE UNIQUE INDEX invite_active_tokens ON invites(hashed_token) WHERE active=TRUE; @@ -45,7 +45,7 @@ CREATE TABLE aliases ( member_id INTEGER NOT NULL, signature BLOB NOT NULL, - FOREIGN KEY ( member_id ) REFERENCES members( "id" ) + FOREIGN KEY ( member_id ) REFERENCES members( "id" ) ON DELETE CASCADE ); CREATE UNIQUE INDEX aliases_ids ON aliases(id); CREATE UNIQUE INDEX aliases_names ON aliases(name); diff --git a/roomdb/sqlite/migrations/03-siwssb-tokens.sql b/roomdb/sqlite/migrations/03-siwssb-tokens.sql index 5163c4f..834846d 100644 --- a/roomdb/sqlite/migrations/03-siwssb-tokens.sql +++ b/roomdb/sqlite/migrations/03-siwssb-tokens.sql @@ -6,10 +6,10 @@ CREATE TABLE SIWSSB_sessions ( member_id INTEGER NOT NULL, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, - FOREIGN KEY ( member_id ) REFERENCES members( "id" ) + FOREIGN KEY ( member_id ) REFERENCES members( "id" ) ON DELETE CASCADE ); CREATE UNIQUE INDEX SIWSSB_by_token ON SIWSSB_sessions(token); -CREATE UNIQUE INDEX SIWSSB_by_member ON SIWSSB_sessions(member_id); +CREATE INDEX SIWSSB_by_member ON SIWSSB_sessions(member_id); -- +migrate Down DROP TABLE SIWSSB_sessions; diff --git a/web/assets/events-demo.js b/web/assets/login-events.js similarity index 83% rename from web/assets/events-demo.js rename to web/assets/login-events.js index 0ee4337..da78109 100644 --- a/web/assets/events-demo.js +++ b/web/assets/login-events.js @@ -1,5 +1,5 @@ -// get the challange from out of the HTML -let sc = document.querySelector("#challange").attributes.ch.value +// get the challenge from out of the HTML +let sc = document.querySelector("#challenge").attributes.ch.value var evtSource = new EventSource(`/sse/events?sc=${sc}`); var ping = document.querySelector('#ping'); diff --git a/web/handlers/aliases.go b/web/handlers/aliases.go index e4f8485..52f8ff6 100644 --- a/web/handlers/aliases.go +++ b/web/handlers/aliases.go @@ -48,7 +48,7 @@ func (a aliasHandler) resolve(rw http.ResponseWriter, req *http.Request) { alias, err := a.db.Resolve(req.Context(), name) if err != nil { - ar.SendError(err) + ar.SendError(fmt.Errorf("aliases: failed to resolve name %q: %w", name, err)) return } diff --git a/web/handlers/auth/handler.go b/web/handlers/auth/handler.go index 785cdf3..4cad0e5 100644 --- a/web/handlers/auth/handler.go +++ b/web/handlers/auth/handler.go @@ -38,6 +38,4 @@ func NewFallbackPasswordHandler( // hook up the auth handler to the router m.Get(router.AuthFallbackSignIn).HandlerFunc(ah.Authorize) - - m.Get(router.AuthSignOut).HandlerFunc(ah.Logout) } diff --git a/web/handlers/auth/withssb.go b/web/handlers/auth/withssb.go index be969e3..4763a46 100644 --- a/web/handlers/auth/withssb.go +++ b/web/handlers/auth/withssb.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "net/url" + "strings" "time" kitlog "github.com/go-kit/kit/log" @@ -72,7 +73,7 @@ func NewWithSSBHandler( m.Get(router.AuthWithSSBSignIn).HandlerFunc(r.HTML("auth/withssb_sign_in.tmpl", ssb.login)) - m.HandleFunc("/sse/login/{sc}", r.HTML("auth/withssb_server_start.tmpl", ssb.startWithServer)) + m.HandleFunc("/sse/login", r.HTML("auth/withssb_server_start.tmpl", ssb.startWithServer)) m.HandleFunc("/sse/events", ssb.eventSource) return &ssb @@ -86,9 +87,6 @@ func (h WithSSBHandler) login(w http.ResponseWriter, req *http.Request) (interfa // validate and update client challenge cc := queryParams.Get("cc") - if _, err := signinwithssb.DecodeChallengeString(cc); err != nil { - return nil, weberrors.ErrBadRequest{Where: "client-challenge", Details: err} - } clientReq.ClientChallenge = cc // check who the client is @@ -110,10 +108,11 @@ func (h WithSSBHandler) login(w http.ResponseWriter, req *http.Request) (interfa // check that we have that member member, err := h.membersdb.GetByFeed(req.Context(), client) if err != nil { + errMsg := fmt.Errorf("sign-in with ssb: client isnt a member: %w", err) if err == roomdb.ErrNotFound { - return nil, weberrors.ErrForbidden{Details: fmt.Errorf("sign-in: client isnt a member")} + return nil, weberrors.ErrForbidden{Details: errMsg} } - return nil, err + return nil, errMsg } clientReq.ClientID = client @@ -134,13 +133,14 @@ func (h WithSSBHandler) login(w http.ResponseWriter, req *http.Request) (interfa var solution string err = edp.Async(ctx, &solution, muxrpc.TypeString, muxrpc.Method{"httpAuth", "requestSolution"}, sc, cc) if err != nil { - return nil, err + return nil, fmt.Errorf("sign-in with ssb: could not request solution from client: %w", err) } // decode and validate the response - solutionBytes, err := base64.URLEncoding.DecodeString(solution) + solution = strings.TrimSuffix(solution, ".sig.ed25519") + solutionBytes, err := base64.StdEncoding.DecodeString(solution) if err != nil { - return nil, err + return nil, fmt.Errorf("sign-in with ssb: failed to decode solution: %w", err) } if !clientReq.Validate(solutionBytes) { @@ -150,17 +150,20 @@ func (h WithSSBHandler) login(w http.ResponseWriter, req *http.Request) (interfa // create a session for invalidation tok, err := h.sessiondb.CreateToken(req.Context(), member.ID) if err != nil { + err = fmt.Errorf("sign-in with ssb: could not create token: %w", err) return nil, err } session, err := h.cookieStore.Get(req, siwssbSessionName) if err != nil { + err = fmt.Errorf("sign-in with ssb: failed to load cookie session: %w", err) return nil, err } session.Values[memberToken] = tok session.Values[userTimeout] = time.Now().Add(lifetime) if err := session.Save(req, w); err != nil { + err = fmt.Errorf("sign-in with ssb: failed to update cookie session: %w", err) return nil, err } @@ -185,19 +188,6 @@ const ( const lifetime = time.Hour * 24 -// Authenticate calls the next unless AuthenticateRequest returns an error -func (h WithSSBHandler) Authenticate(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if _, err := h.AuthenticateRequest(r); err != nil { - // TODO: render.Error - http.Error(w, weberrors.ErrNotAuthorized.Error(), http.StatusForbidden) - return - } - - next.ServeHTTP(w, r) - }) -} - // AuthenticateRequest uses the passed request to load and return the session data that was stored previously. // If it is invalid or there is no session, it will return ErrNotAuthorized. // Otherwise it will return the member ID that belongs to the session. @@ -249,26 +239,35 @@ func (h WithSSBHandler) AuthenticateRequest(r *http.Request) (*roomdb.Member, er } // Logout destroys the session data and updates the cookie with an invalidated one. -func (h WithSSBHandler) Logout(w http.ResponseWriter, r *http.Request) { +func (h WithSSBHandler) Logout(w http.ResponseWriter, r *http.Request) error { session, err := h.cookieStore.Get(r, siwssbSessionName) if err != nil { - // TODO: render.Error - http.Error(w, err.Error(), http.StatusInternalServerError) - // ah.errorHandler(w, r, err, http.StatusInternalServerError) - return + return err + } + + tokenVal, ok := session.Values[memberToken] + if !ok { + // not a sign-in with ssb session + return nil + } + + token, ok := tokenVal.(string) + if !ok { + return fmt.Errorf("wrong token type: %T", tokenVal) + } + + err = h.sessiondb.RemoveToken(r.Context(), token) + if err != nil { + return err } session.Values[userTimeout] = time.Now().Add(-lifetime) session.Options.MaxAge = -1 if err := session.Save(r, w); err != nil { - // TODO: render.Error - http.Error(w, err.Error(), http.StatusInternalServerError) - // ah.errorHandler(w, r, err, http.StatusInternalServerError) - return + return err } - http.Redirect(w, r, "/", http.StatusSeeOther) - return + return nil } // server-sent-events stuff @@ -278,24 +277,26 @@ func (h WithSSBHandler) startWithServer(w http.ResponseWriter, req *http.Request var queryParams = make(url.Values) queryParams.Set("action", "start-http-auth") + queryParams.Set("sid", h.roomID.Ref()) + queryParams.Set("sc", sc) var startAuthURI url.URL startAuthURI.Scheme = "ssb" startAuthURI.Opaque = "experimental" startAuthURI.RawQuery = queryParams.Encode() - qrCode, err := qrcode.New(startAuthURI.String(), qrcode.High) + // generate a QR code with the token inside so that you can open it easily in a supporting mobile app + qrCode, err := qrcode.New(startAuthURI.String(), qrcode.Medium) if err != nil { return nil, err } qrCode.BackgroundColor = color.RGBA{R: 0xf9, G: 0xfa, B: 0xfb} qrCode.ForegroundColor = color.Black - qrCodeData, err := qrCode.PNG(-8) + qrCodeData, err := qrCode.PNG(-5) if err != nil { return nil, err } - qrURI := "data:image/png;base64," + base64.StdEncoding.EncodeToString(qrCodeData) return struct { @@ -383,7 +384,7 @@ func (h WithSSBHandler) eventSource(w http.ResponseWriter, r *http.Request) { case update := <-evtCh: evt := event{ ID: evtID, - Data: "challange validation failed", + Data: "challenge validation failed", Event: "failed", } diff --git a/web/handlers/auth_test.go b/web/handlers/auth_test.go index 673b763..48c4f5f 100644 --- a/web/handlers/auth_test.go +++ b/web/handlers/auth_test.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package handlers import ( @@ -193,9 +195,9 @@ func TestAuthWithSSBNotConnected(t *testing.T) { urlTo := web.NewURLTo(ts.Router) - signInStartURL := urlTo(router.AuthWithSSBSignIn, + signInStartURL := urlTo(router.AuthLogin, "cid", client.Feed.Ref(), - "challenge", cc, + "cc", cc, ) r.NotNil(signInStartURL) @@ -224,9 +226,9 @@ func TestAuthWithSSBNotAllowed(t *testing.T) { urlTo := web.NewURLTo(ts.Router) - signInStartURL := urlTo(router.AuthWithSSBSignIn, + signInStartURL := urlTo(router.AuthLogin, "cid", client.Feed.Ref(), - "challenge", cc, + "cc", cc, ) r.NotNil(signInStartURL) @@ -293,7 +295,7 @@ func TestAuthWithSSBHasClient(t *testing.T) { // sign the request now that we have the sc clientSig := req.Sign(client.Pair.Secret) - *strptr = base64.URLEncoding.EncodeToString(clientSig) + *strptr = base64.StdEncoding.EncodeToString(clientSig) return nil }) @@ -305,7 +307,8 @@ func TestAuthWithSSBHasClient(t *testing.T) { req.ClientChallenge = cc // prepare the url - signInStartURL := web.NewURLTo(ts.Router)(router.AuthWithSSBSignIn, + urlTo := web.NewURLTo(ts.Router) + signInStartURL := urlTo(router.AuthLogin, "cid", client.Feed.Ref(), "cc", cc, ) @@ -316,7 +319,11 @@ func TestAuthWithSSBHasClient(t *testing.T) { t.Log(signInStartURL.String()) doc, resp := ts.Client.GetHTML(signInStartURL.String()) - a.Equal(http.StatusOK, resp.Code) + a.Equal(http.StatusTemporaryRedirect, resp.Code) + + dashboardURL, err := ts.Router.Get(router.AdminDashboard).URL() + r.Nil(err) + a.Equal(dashboardURL.Path, resp.Header().Get("Location")) webassert.Localized(t, doc, []webassert.LocalizedElement{ // {"#welcome", "AuthWithSSBWelcome"}, @@ -337,8 +344,7 @@ func TestAuthWithSSBHasClient(t *testing.T) { jar.SetCookies(signInStartURL, sessionCookie) // now request the protected dashboard page - dashboardURL, err := ts.Router.Get(router.AdminDashboard).URL() - r.Nil(err) + dashboardURL.Host = "localhost" dashboardURL.Scheme = "https" diff --git a/web/handlers/http.go b/web/handlers/http.go index c7b746c..b7485cd 100644 --- a/web/handlers/http.go +++ b/web/handlers/http.go @@ -11,6 +11,7 @@ import ( "strconv" "time" + "github.com/go-kit/kit/log/level" "github.com/gorilla/csrf" "github.com/gorilla/sessions" "github.com/russross/blackfriday/v2" @@ -245,6 +246,14 @@ func New( // just hooks up the router to the handler roomsAuth.NewFallbackPasswordHandler(m, r, authWithPassword) + m.Get(router.AuthSignOut).HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + err = authWithSSB.Logout(w, req) + if err != nil { + level.Warn(logging.FromContext(req.Context())).Log("err", err) + } + authWithPassword.Logout(w, req) + }) + adminHandler := admin.Handler( netInfo.Domain, r, diff --git a/web/handlers/setup_test.go b/web/handlers/setup_test.go index b5bb49b..578d603 100644 --- a/web/handlers/setup_test.go +++ b/web/handlers/setup_test.go @@ -16,15 +16,16 @@ import ( "github.com/gorilla/mux" "go.mindeco.de/http/tester" "go.mindeco.de/logging/logtest" - refs "go.mindeco.de/ssb-refs" "github.com/ssb-ngi-pointer/go-ssb-room/internal/network/mocked" "github.com/ssb-ngi-pointer/go-ssb-room/internal/repo" + "github.com/ssb-ngi-pointer/go-ssb-room/internal/signinwithssb" "github.com/ssb-ngi-pointer/go-ssb-room/roomdb" "github.com/ssb-ngi-pointer/go-ssb-room/roomdb/mockdb" "github.com/ssb-ngi-pointer/go-ssb-room/roomstate" "github.com/ssb-ngi-pointer/go-ssb-room/web/i18n" "github.com/ssb-ngi-pointer/go-ssb-room/web/router" + refs "go.mindeco.de/ssb-refs" ) type testSession struct { @@ -46,6 +47,8 @@ type testSession struct { MockedEndpoints *mocked.FakeEndpoints + SignalBridge *signinwithssb.SignalBridge + NetworkInfo NetworkInfo } @@ -98,12 +101,15 @@ func setup(t *testing.T) *testSession { ts.Router = router.CompleteApp() + ts.SignalBridge = signinwithssb.NewSignalBridge() + h, err := New( log, testRepo, ts.NetworkInfo, ts.RoomState, ts.MockedEndpoints, + ts.SignalBridge, Databases{ Aliases: ts.AliasesDB, AuthFallback: ts.AuthFallbackDB, diff --git a/web/i18n/defaults/active.en.toml b/web/i18n/defaults/active.en.toml index fc26a25..b944025 100644 --- a/web/i18n/defaults/active.en.toml +++ b/web/i18n/defaults/active.en.toml @@ -4,6 +4,7 @@ GenericSave = "Save" GenericCreate = "Create" GenericPreview = "Preview" GenericLanguage = "Language" +GenericOpenLink = "Open Link" PageNotFound = "The requested page was not found." @@ -20,7 +21,7 @@ AuthSignIn = "Sign in" AuthSignOut = "Sign out" AuthWithSSBTitle = "Sign-in with SSB" -AuthWithSSBWelcome = "If you have a compatible device/application, you can sign-in here without a password." +AuthWithSSBWelcome = "If you have a compatible device/application, you can sign-in here without a password. Open the QR-Code on your mobile device to complete the process or click the link below." AdminDashboardWelcome = "Welcome to your dashboard" AdminDashboardTitle = "Room Admin Dashboard" diff --git a/web/members/helper.go b/web/members/helper.go index 0dd66a7..404bd8a 100644 --- a/web/members/helper.go +++ b/web/members/helper.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + // Package members implements helpers for accessing the currently logged in admin or moderator of an active request. package members diff --git a/web/members/testing.go b/web/members/testing.go index 696fc51..6f31f1b 100644 --- a/web/members/testing.go +++ b/web/members/testing.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: MIT + package members import ( diff --git a/web/router/auth.go b/web/router/auth.go index d74e3d0..02d3323 100644 --- a/web/router/auth.go +++ b/web/router/auth.go @@ -9,23 +9,23 @@ const ( AuthFallbackSignInForm = "auth:fallback:signin:form" AuthFallbackSignIn = "auth:fallback:signin" - AuthWithSSBSignIn = "auth:ssb:signin" + AuthWithSSBSignIn = "auth:ssb:login" + // AuthWithSSBSignIn AuthSignOut = "auth:logout" ) -// NewSignin constructs a mux.Router containing the routes for sign-in and -out +// Auth constructs a mux.Router containing the routes for sign-in and -out func Auth(m *mux.Router) *mux.Router { if m == nil { m = mux.NewRouter() } - // register fallback - m.Path("/fallback/signin").Methods("GET").Name(AuthFallbackSignInForm) - m.Path("/fallback/signin").Methods("POST").Name(AuthFallbackSignIn) - - m.Path("/withssb/signin").Methods("GET").Name(AuthWithSSBSignIn) + // register password fallback + m.Path("/password/signin").Methods("GET").Name(AuthFallbackSignInForm) + m.Path("/password/signin").Methods("POST").Name(AuthFallbackSignIn) + m.Path("/login").Methods("GET").Name(AuthWithSSBSignIn) m.Path("/logout").Methods("GET").Name(AuthSignOut) return m diff --git a/web/router/complete.go b/web/router/complete.go index 847473c..eaa838e 100644 --- a/web/router/complete.go +++ b/web/router/complete.go @@ -24,13 +24,13 @@ const ( func CompleteApp() *mux.Router { m := mux.NewRouter() - Auth(m.PathPrefix("/auth").Subrouter()) + Auth(m) Admin(m.PathPrefix("/admin").Subrouter()) m.Path("/").Methods("GET").Name(CompleteIndex) m.Path("/about").Methods("GET").Name(CompleteAbout) - m.Path("/{alias}").Methods("GET").Name(CompleteAliasResolve) + m.Path("/alias/{alias}").Methods("GET").Name(CompleteAliasResolve) m.Path("/invite/accept").Methods("GET").Name(CompleteInviteAccept) m.Path("/invite/consume").Methods("POST").Name(CompleteInviteConsume) diff --git a/web/templates/admin/members-remove-confirm.tmpl b/web/templates/admin/members-remove-confirm.tmpl index 0b17575..a228fa1 100644 --- a/web/templates/admin/members-remove-confirm.tmpl +++ b/web/templates/admin/members-remove-confirm.tmpl @@ -1,11 +1,11 @@ -{{ define "title" }}{{i18n "AdminAllowListRemoveConfirmTitle"}}{{ end }} +{{ define "title" }}{{i18n "AdminMembersRemoveConfirmTitle"}}{{ end }} {{ define "content" }}
{{i18n "AdminAllowListRemoveConfirmWelcome"}} + >{{i18n "AdminMembersRemoveConfirmWelcome"}} 
{{i18n "AuthWithSSBWelcome"}}
-

TODO: qr code of the code

- QR-Code to pass challange to an App - 
{{.SSBURI}}
+ QR-Code to pass the challenge to an App + {{i18n "GenericOpenLink"}}

Server events

-
- +
+ {{end}} \ No newline at end of file diff --git a/web/templates/auth/withssb_sign_in.tmpl b/web/templates/auth/withssb_sign_in.tmpl index 3931fbb..f75459a 100644 --- a/web/templates/auth/withssb_sign_in.tmpl +++ b/web/templates/auth/withssb_sign_in.tmpl @@ -1,9 +1,6 @@ {{ define "title" }}{{i18n "AuthWithSSBTitle"}}{{ end }} {{ define "content" }} -
-

{{i18n "AuthWithSSBWelcome"}}

-
- 
{{.}}
+ Proceed to Dashboard
{{end}} \ No newline at end of file