From cc905083a26ee10093468d8519ee1d45a69d9fee Mon Sep 17 00:00:00 2001 From: Florimond Husquinet Date: Fri, 13 Dec 2019 06:51:19 +0100 Subject: [PATCH] Fix presence authorization + fix receiving presence events with an extension key. --- .vscode/settings.json | 1 - internal/broker/handlers.go | 34 +++++++++++--------------------- internal/broker/handlers_test.go | 22 ++++++++++++++------- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index b5903463..cf33d351 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -11,7 +11,6 @@ "appveyor.yml": true, "Dockerfile": true }, - "editor.fontFamily": "Fira Code", "editor.fontLigatures": true, "go.formatTool": "gofmt", "go.formatFlags": [ diff --git a/internal/broker/handlers.go b/internal/broker/handlers.go index 4540408d..925d22bc 100644 --- a/internal/broker/handlers.go +++ b/internal/broker/handlers.go @@ -406,38 +406,28 @@ func (c *Conn) onPresence(payload []byte) (response, bool) { if err := json.Unmarshal(payload, &msg); err != nil { return errors.ErrBadRequest, false } - - // Attempt to parse the key, this should be a master key - key, err := c.keys.DecryptKey(msg.Key) - if err != nil || !key.HasPermission(security.AllowPresence) || key.IsExpired() { - return errors.ErrUnauthorized, false - } - - // Attempt to fetch the contract using the key. Underneath, it's cached. - contract, contractFound := c.service.contracts.Get(key.Contract()) - if !contractFound { - return errors.ErrNotFound, false - } - - // Validate the contract - if !contract.Validate(key) { - return errors.ErrUnauthorized, false - } - // Ensure we have trailing slash if !strings.HasSuffix(msg.Channel, "/") { msg.Channel = msg.Channel + "/" } - // Parse the channel - channel := security.ParseChannel([]byte("emitter/" + msg.Channel)) + channel := security.ParseChannel([]byte(msg.Key + "/" + msg.Channel)) if channel.ChannelType == security.ChannelInvalid { return errors.ErrBadRequest, false } + // Check the authorization and permissions + _, key, allowed := c.service.authorize(channel, security.AllowPresence) + if !allowed { + return errors.ErrUnauthorized, false + } + + // Keys which are supposed to be extended should not be used for presence + if key.HasPermission(security.AllowExtend) { + return errors.ErrUnauthorizedExt, false + } // Create the ssid for the presence ssid := message.NewSsid(key.Contract(), channel.Query) - // Check if the client is interested in subscribing/unsubscribing from changes. if msg.Changes != nil { if *msg.Changes { @@ -446,12 +436,10 @@ func (c *Conn) onPresence(payload []byte) (response, bool) { c.Unsubscribe(message.NewSsidForPresence(ssid), nil) } } - // If we requested a status, populate the slice via scatter/gather. now := time.Now().UTC().Unix() who := make([]presenceInfo, 0, 4) if msg.Status { - // Gather local & cluster presence who = append(who, getAllPresence(c.service, ssid)...) return &presenceResponse{ diff --git a/internal/broker/handlers_test.go b/internal/broker/handlers_test.go index 76b8b53d..24cf49f4 100644 --- a/internal/broker/handlers_test.go +++ b/internal/broker/handlers_test.go @@ -358,7 +358,7 @@ func TestHandlers_onPresence(t *testing.T) { // TODO : // - valid key for the right channel, but no presence right. // - test Who - license, _ := license.Parse(testLicense) + license, _ := license.Parse(testLicenseV2) tests := []struct { channel string payload string @@ -371,7 +371,7 @@ func TestHandlers_onPresence(t *testing.T) { }{ { channel: "emitter/presence/", - payload: "{\"key\":\"VfW_Cv5wWVZPHgCvLwJAuU2bgRFKXQEY\",\"channel\":\"a\",\"status\":true}", + payload: "{\"key\":\"hw7Jv3TMhYTg6lLk2fQoSvs2EP3gjFPk\",\"channel\":\"a\",\"status\":true}", contractValid: true, contractFound: true, success: true, @@ -390,7 +390,7 @@ func TestHandlers_onPresence(t *testing.T) { }, { channel: "emitter/presence/", - payload: "{\"key\":\"VfW_Cv5wWVZPHgCvLwJAuU2bgRFKXQEY\",\"channel\":\"a+b\",\"status\":true}", + payload: "{\"key\":\"hw7Jv3TMhYTg6lLk2fQoSvs2EP3gjFPk\",\"channel\":\"a+b\",\"status\":true}", contractValid: true, contractFound: true, success: false, @@ -399,7 +399,7 @@ func TestHandlers_onPresence(t *testing.T) { }, { channel: "emitter/presence/", - payload: "{\"key\":\"0Nq8SWbL8qoOKEDqh_ebBZRqJDby30m\",\"channel\":\"a\",\"status\":true}", + payload: "{\"key\":\"07XJv3TMhYTg6lLk2fQoSift1AbgjFPk\",\"channel\":\"a\",\"status\":true}", contractValid: true, contractFound: true, success: false, @@ -408,20 +408,28 @@ func TestHandlers_onPresence(t *testing.T) { }, { channel: "emitter/presence/", - payload: "{\"key\":\"VfW_Cv5wWVZPHgCvLwJAuU2bgRFKXQEY\",\"channel\":\"a+b\",\"status\":true}", - err: errors.ErrNotFound, + payload: "{\"key\":\"hw7Jv3TMhYTg6lLk2fQoSvs2EP3gjFPk\",\"channel\":\"a\",\"status\":true}", + err: errors.ErrUnauthorized, contractValid: true, contractFound: false, msg: "Contract not found case", }, { channel: "emitter/presence/", - payload: "{\"key\":\"VfW_Cv5wWVZPHgCvLwJAuU2bgRFKXQEY\",\"channel\":\"a+b\",\"status\":true}", + payload: "{\"key\":\"hw7Jv3TMhYTg6lLk2fQoSvs2EP3gjFPk\",\"channel\":\"a\",\"status\":true}", err: errors.ErrUnauthorized, contractValid: false, contractFound: true, msg: "Contract is invalid case", }, + { + channel: "emitter/presence/", + payload: "{\"key\":\"sVTJv3TMhYTg6lLk2fQoCvs2EP3gjFPk\",\"channel\":\"a\",\"status\":true}", + err: errors.ErrUnauthorizedExt, + contractValid: true, + contractFound: true, + msg: "Extended key is unauthorized case", + }, } for _, tc := range tests {