From de3a016f4dd2cbe3fb19427e922f8489bb4ea665 Mon Sep 17 00:00:00 2001 From: Juliusz Chroboczek Date: Sun, 20 Feb 2022 15:32:18 +0100 Subject: [PATCH] Set the username in the server when using tokens. This avoids the need to pass the username in the URL without requiring the client to parse tokens. --- README.PROTOCOL | 38 +++++++++++++++++++++++++++++--------- diskwriter/diskwriter.go | 4 ++++ galene-link/galene-link.go | 3 --- group/client.go | 1 + group/group.go | 33 ++++++++++++++++----------------- group/group_test.go | 11 ++++++----- rtpconn/webclient.go | 4 ++++ static/galene.js | 15 +++++++-------- token/token.go | 27 +++++++++++++++------------ token/token_test.go | 16 +++++++--------- webserver/webserver.go | 2 +- 11 files changed, 90 insertions(+), 64 deletions(-) diff --git a/README.PROTOCOL b/README.PROTOCOL index 64d1853..9b302f7 100644 --- a/README.PROTOCOL +++ b/README.PROTOCOL @@ -117,8 +117,8 @@ The `join` message requests that the sender join or leave a group: } ``` -If token-based authorisation is beling used, then the `password` field is -omitted, and a `token` field is included instead. +If token-based authorisation is beling used, then the `username` and +`password` fields are omitted, and a `token` field is included instead. When the sender has effectively joined the group, the peer will send a 'joined' message of kind 'join'; it may then send a 'joined' message of @@ -138,10 +138,11 @@ its permissions or in the recommended RTC configuration. } ``` -The `permissions` field is an array of strings that may contain the values -`present`, `op` and `record`. The `status` field is a dictionary that -contains status information about the group, in the same format as at the -`.status.json` URL above. +The `username` field is the username that the server assigned to this +user. The `permissions` field is an array of strings that may contain the +values `present`, `op` and `record`. The `status` field is a dictionary +that contains status information about the group, in the same format as at +the `.status.json` URL above. ## Maintaining group membership @@ -366,13 +367,24 @@ Currently defined kinds include `clearchat` (not to be confused with the # Authorisation protocol +In addition to username/password authentication, Galene supports +authentication using cryptographic tokens. Two flows are supported: using +an authentication server, where Galene's client requests a token from +a third-party server, and using an authentication portal, where +a third-party login portal redirects the user to Galene. Authentication +servers are somewhat simpler to implement, but authentication portals are +more flexible and avoid communicating the user's password to Galene's +Javascript code. + +## Authentication server + If a group's status dictionary has a non-empty `authServer` field, then -the group uses token authentication. Before joining, the client sends +the group uses an authentication server. Before joining, the client sends a POST request to the authorisation server URL containing in its body a JSON dictionary of the following form: ```javascript { - "location": "https://galene.example.org/group/groupname", + "location": "https://galene.example.org/group/groupname/", "username": username, "password": password } @@ -384,7 +396,7 @@ allowed to join, then the authorisation server replies with a signed JWT ```javascript { "sub": username, - "aud": "https://galene.example.org/group/groupname", + "aud": "https://galene.example.org/group/groupname/", "permissions": ["present"], "iat": now, "exp": now + 30s, @@ -396,4 +408,12 @@ the same format as in the `joined` message. Since the client will only use the token once, at the very beginning of the session, the tokens issued may have a short lifetime (on the order of 30s). +## Authentication portal +If a group's status dictionary has a non-empty `authPortal` field, Galene +redirects the user agent to the URL indicated by `authPortal`. The +authentication portal performs authorisation, generates a token as above, +then redirects back to the group's URL with the token stores in a URL +query parameter named `token`: + + https://galene.example.org/group/groupname/?token=eyJhbG... diff --git a/diskwriter/diskwriter.go b/diskwriter/diskwriter.go index 713c80b..8f3c720 100644 --- a/diskwriter/diskwriter.go +++ b/diskwriter/diskwriter.go @@ -59,6 +59,10 @@ func (client *Client) Username() string { return "RECORDING" } +func (client *Client) SetUsername(string) { + return +} + func (client *Client) SetPermissions(perms []string) { return } diff --git a/galene-link/galene-link.go b/galene-link/galene-link.go index 7de5b3b..cfc1bda 100644 --- a/galene-link/galene-link.go +++ b/galene-link/galene-link.go @@ -100,9 +100,6 @@ func main() { fmt.Println(s) } else { query := url.Values{} - if username != "" { - query.Add("username", username) - } query.Add("token", s) outURL := &url.URL{ Scheme: groupURL.Scheme, diff --git a/group/client.go b/group/client.go index ab5e19c..dfc9a6f 100644 --- a/group/client.go +++ b/group/client.go @@ -92,6 +92,7 @@ type Client interface { Group() *Group Id() string Username() string + SetUsername(string) Permissions() []string SetPermissions([]string) Data() map[string]interface{} diff --git a/group/group.go b/group/group.go index 724c8a8..37d75dc 100644 --- a/group/group.go +++ b/group/group.go @@ -565,11 +565,12 @@ func AddClient(group string, c Client, creds ClientCredentials) (*Group, error) clients := g.getClientsUnlocked(nil) if !member("system", c.Permissions()) { - perms, err := g.description.GetPermission(group, creds) + username, perms, err := g.description.GetPermission(group, creds) if err != nil { return nil, err } + c.SetUsername(username) c.SetPermissions(perms) if !member("op", perms) { @@ -1078,9 +1079,9 @@ func GetDescription(name string) (*Description, error) { return &desc, nil } -func (desc *Description) GetPermission(group string, creds ClientCredentials) ([]string, error) { +func (desc *Description) GetPermission(group string, creds ClientCredentials) (string, []string, error) { if !desc.AllowAnonymous && creds.Username == "" { - return nil, ErrAnonymousNotAuthorised + return "", nil, ErrAnonymousNotAuthorised } if creds.Token == "" { @@ -1091,36 +1092,34 @@ func (desc *Description) GetPermission(group string, creds ClientCredentials) ([ if desc.AllowRecording { p = append(p, "record") } - return p, nil + return creds.Username, p, nil } - return nil, ErrNotAuthorised + return "", nil, ErrNotAuthorised } if found, good := matchClient(group, creds, desc.Presenter); found { if good { - return []string{"present"}, nil + return creds.Username, []string{"present"}, nil } - return nil, ErrNotAuthorised + return "", nil, ErrNotAuthorised } if found, good := matchClient(group, creds, desc.Other); found { if good { - return nil, nil + return creds.Username, nil, nil } - return nil, ErrNotAuthorised + return "", nil, ErrNotAuthorised } - return nil, ErrNotAuthorised + return "", nil, ErrNotAuthorised } - aud, perms, err := token.Valid( - creds.Username, creds.Token, desc.AuthKeys, - ) + sub, aud, perms, err := token.Valid(creds.Token, desc.AuthKeys) if err != nil { log.Printf("Token authentication: %v", err) - return nil, ErrNotAuthorised + return "", nil, ErrNotAuthorised } conf, err := GetConfiguration() if err != nil { log.Printf("Read config.json: %v", err) - return nil, err + return "", nil, err } ok := false for _, u := range aud { @@ -1145,9 +1144,9 @@ func (desc *Description) GetPermission(group string, creds ClientCredentials) ([ } } if !ok { - return nil, ErrNotAuthorised + return "", nil, ErrNotAuthorised } - return perms, nil + return sub, perms, nil } type Status struct { diff --git a/group/group_test.go b/group/group_test.go index f0354a9..4d925f2 100644 --- a/group/group_test.go +++ b/group/group_test.go @@ -167,7 +167,7 @@ func TestPermissions(t *testing.T) { for _, c := range badClients { t.Run("bad "+c.Username, func(t *testing.T) { - p, err := d.GetPermission("test", c) + _, p, err := d.GetPermission("test", c) if err != ErrNotAuthorised { t.Errorf("GetPermission %v: %v %v", c, err, p) } @@ -176,12 +176,13 @@ func TestPermissions(t *testing.T) { for _, cp := range goodClients { t.Run("good "+cp.c.Username, func(t *testing.T) { - p, err := d.GetPermission("test", cp.c) + u, p, err := d.GetPermission("test", cp.c) if err != nil { t.Errorf("GetPermission %v: %v", cp.c, err) - } else if !reflect.DeepEqual(p, cp.p) { - t.Errorf("%v: got %v, expected %v", - cp.c, p, cp.p) + } else if u != cp.c.Username || + !reflect.DeepEqual(p, cp.p) { + t.Errorf("%v: got %v %v, expected %v", + cp.c, u, p, cp.p) } }) } diff --git a/rtpconn/webclient.go b/rtpconn/webclient.go index 1916a21..1624567 100644 --- a/rtpconn/webclient.go +++ b/rtpconn/webclient.go @@ -86,6 +86,10 @@ func (c *webClient) Username() string { return c.username } +func (c *webClient) SetUsername(username string) { + c.username = username +} + func (c *webClient) Permissions() []string { return c.permissions } diff --git a/static/galene.js b/static/galene.js index 9b1fca0..dbd6df7 100644 --- a/static/galene.js +++ b/static/galene.js @@ -29,9 +29,6 @@ let serverConnection; /** @type {Object} */ let groupStatus = {}; -/** @type {string} */ -let username = null; - /** @type {string} */ let token = null; @@ -273,8 +270,11 @@ function setConnected(connected) { } } -/** @this {ServerConnection} */ -async function gotConnected() { +/** + * @this {ServerConnection} + * @param {string} [username] + */ +async function gotConnected(username) { let credentials; if(token) { credentials = { @@ -283,9 +283,9 @@ async function gotConnected() { }; token = null; } else { - username = getInputElement('username').value.trim(); setConnected(true); + username = getInputElement('username').value.trim(); let pw = getInputElement('password').value; getInputElement('password').value = ''; if(!groupStatus.authServer) @@ -2139,7 +2139,7 @@ function gotUser(id, kind) { } function displayUsername() { - document.getElementById('userspan').textContent = username; + document.getElementById('userspan').textContent = serverConnection.username; let op = serverConnection.permissions.indexOf('op') >= 0; let present = serverConnection.permissions.indexOf('present') >= 0; let text = ''; @@ -3776,7 +3776,6 @@ async function start() { setMediaChoices(false).then(e => reflectSettings()); if(parms.has('token')) { - username = parms.get('username'); token = parms.get('token'); await serverConnect(); } else if(groupStatus.authPortal) { diff --git a/token/token.go b/token/token.go index cd04876..88407a0 100644 --- a/token/token.go +++ b/token/token.go @@ -10,8 +10,6 @@ import ( "github.com/golang-jwt/jwt/v4" ) -var ErrUnexpectedSub = errors.New("unexpected 'sub' field") - func parseBase64(k string, d map[string]interface{}) ([]byte, error) { v, ok := d[k].(string) if !ok { @@ -117,18 +115,23 @@ func toStringArray(a []interface{}) ([]string, bool) { return b, true } -func Valid(username, token string, keys []map[string]interface{}) ([]string, []string, error) { +func Valid(token string, keys []map[string]interface{}) (string, []string, []string, error) { tok, err := jwt.Parse(token, func(t *jwt.Token) (interface{}, error) { return getKey(t.Header, keys) }) if err != nil { - return nil, nil, err + return "", nil, nil, err } claims := tok.Claims.(jwt.MapClaims) - sub, ok := claims["sub"].(string) - if !ok || sub != username { - return nil, nil, ErrUnexpectedSub + var sub string + if s, ok := claims["sub"]; ok && s != nil { + ss, ok := s.(string) + if !ok { + return "", nil, nil, + errors.New("invalid 'sub' field") + } + sub = ss } var aud []string @@ -139,11 +142,11 @@ func Valid(username, token string, keys []map[string]interface{}) ([]string, []s case []interface{}: aud, ok = toStringArray(a) if !ok { - return nil, nil, + return "", nil, nil, errors.New("invalid 'aud' field") } default: - return nil, nil, + return "", nil, nil, errors.New("invalid 'aud' field") } } @@ -152,14 +155,14 @@ func Valid(username, token string, keys []map[string]interface{}) ([]string, []s if p, ok := claims["permissions"]; ok && p != nil { pp, ok := p.([]interface{}) if !ok { - return nil, nil, + return "", nil, nil, errors.New("invalid 'permissions' field") } perms, ok = toStringArray(pp) if !ok { - return nil, nil, + return "", nil, nil, errors.New("invalid 'permissions' field") } } - return aud, perms, nil + return sub, aud, perms, nil } diff --git a/token/token_test.go b/token/token_test.go index f0c2e4b..689f4a1 100644 --- a/token/token_test.go +++ b/token/token_test.go @@ -69,11 +69,14 @@ func TestValid(t *testing.T) { goodToken := "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJqb2huIiwiYXVkIjoiaHR0cHM6Ly9nYWxlbmUub3JnOjg0NDMvZ3JvdXAvYXV0aC8iLCJwZXJtaXNzaW9ucyI6WyJwcmVzZW50Il0sImlhdCI6MTY0NTMxMDI5NCwiZXhwIjoyOTA2NzUwMjk0LCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjEyMzQvIn0.6xXpgBkBMn4PSBpnwYHb-gRn_Q97Yq9DoKkAf2_6iwc" - aud, perms, err := Valid("john", goodToken, keys) + sub, aud, perms, err := Valid(goodToken, keys) if err != nil { t.Errorf("Token invalid: %v", err) } else { + if sub != "john" { + t.Errorf("Unexpected sub: %v", sub) + } if !reflect.DeepEqual(aud, []string{"https://galene.org:8443/group/auth/"}) { t.Errorf("Unexpected aud: %v", aud) } @@ -82,14 +85,9 @@ func TestValid(t *testing.T) { } } - aud, perms, err = Valid("jack", goodToken, keys) - if err != ErrUnexpectedSub { - t.Errorf("Token should have bad username") - } - badToken := "eyJ0eXAiOiJKV1QiLCJhbGciOiJub25lIn0.eyJzdWIiOiJqb2huIiwiYXVkIjoiaHR0cHM6Ly9nYWxlbmUub3JnOjg0NDMvZ3JvdXAvYXV0aC8iLCJwZXJtaXNzaW9ucyI6WyJwcmVzZW50Il0sImlhdCI6MTY0NTMxMDQ2OSwiZXhwIjoyOTA2NzUwNDY5LCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjEyMzQvIn0." - _, _, err = Valid("john", badToken, keys) + _, _, _, err = Valid(badToken, keys) var verr *jwt.ValidationError if !errors.As(err, &verr) { @@ -98,14 +96,14 @@ func TestValid(t *testing.T) { expiredToken := "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJqb2huIiwiYXVkIjoiaHR0cHM6Ly9nYWxlbmUub3JnOjg0NDMvZ3JvdXAvYXV0aC8iLCJwZXJtaXNzaW9ucyI6WyJwcmVzZW50Il0sImlhdCI6MTY0NTMxMDMyMiwiZXhwIjoxNjQ1MzEwMzUyLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjEyMzQvIn0.jyqRhoV6iK54SvlP33Fy630aDo-sLNmKKi1kcfqs378" - _, _, err = Valid("john", expiredToken, keys) + _, _, _, err = Valid(expiredToken, keys) if !errors.As(err, &verr) { t.Errorf("Token should be expired") } noneToken := "eyJ0eXAiOiJKV1QiLCJhbGciOiJub25lIn0.eyJzdWIiOiJqb2huIiwiYXVkIjoiaHR0cHM6Ly9nYWxlbmUub3JnOjg0NDMvZ3JvdXAvYXV0aC8iLCJwZXJtaXNzaW9ucyI6WyJwcmVzZW50Il0sImlhdCI6MTY0NTMxMDQwMSwiZXhwIjoxNjQ1MzEwNDMxLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjEyMzQvIn0." - _, _, err = Valid("john", noneToken, keys) + _, _, _, err = Valid(noneToken, keys) if err == nil { t.Errorf("Unsigned token should fail") } diff --git a/webserver/webserver.go b/webserver/webserver.go index a3fce22..3dba39c 100644 --- a/webserver/webserver.go +++ b/webserver/webserver.go @@ -586,7 +586,7 @@ func checkGroupPermissions(w http.ResponseWriter, r *http.Request, groupname str return false } - p, err := desc.GetPermission(groupname, + _, p, err := desc.GetPermission(groupname, group.ClientCredentials{ Username: user, Password: pass,