Skip to content

Commit

Permalink
Revoke NC token on logout and set defaults
Browse files Browse the repository at this point in the history
Signed-off-by: Piotr Jurkiewicz <piotr.jurkiewicz@nordsec.com>
  • Loading branch information
piotrjurkiewicz committed May 13, 2024
1 parent 43ade03 commit 89720eb
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 11 deletions.
54 changes: 54 additions & 0 deletions core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ const (

type CredentialsAPI interface {
NotificationCredentials(token, appUserID string) (NotificationCredentialsResponse, error)
NotificationCredentialsRevoke(token, appUserID string, purgeSession bool) (NotificationCredentialsRevokeResponse, error)
ServiceCredentials(string) (*CredentialsResponse, error)
TokenRenew(string) (*TokenRenewResponse, error)
Services(string) (ServicesResponse, error)
Expand Down Expand Up @@ -406,6 +407,15 @@ type NotificationCredentialsResponse struct {
Password string `json:"password"`
}

type NotificationCredentialsRevokeRequest struct {
AppUserID string `json:"app_user_uid"`
PurgeSession bool `json:"purge_session"`
}

type NotificationCredentialsRevokeResponse struct {
Status string `json:"status"`
}

// NotificationCredentials retrieves the credentials for notification center appUserID
func (api *DefaultAPI) NotificationCredentials(token, appUserID string) (NotificationCredentialsResponse, error) {
data, err := json.Marshal(NotificationCredentialsRequest{
Expand Down Expand Up @@ -436,6 +446,50 @@ func (api *DefaultAPI) NotificationCredentials(token, appUserID string) (Notific
return resp, nil
}

// NotificationCredentialsRevoke revokes the credentials for notification center appUserID
func (api *DefaultAPI) NotificationCredentialsRevoke(token, appUserID string, purgeSession bool) (NotificationCredentialsRevokeResponse, error) {
// Calling tokens/revoke endpoint with just bearer token will revoke user credentials for every user device.
// For example, if user has VPN app for android/iOS/mac, whatever, all of his/her devices will be disconnected.
// If you provide additionally app_user_id, then only credential for specific app/device will be revoked.
// Connection on other devices will stay unaffected.
// The purge_session param make sense only in cases when you definitely know, that app_user_id was generated
// just for one time usage and after this usage it is not needed anymore. The good example is exactly tests,
// where on each run you generate different app_user_id. In usual scenarios in ideal case, app_user_id on the
// same device for the same user and same app should stay constant, even if app were reinstalled. So there is no
// need to use purge_session at all, and it even can have a bad consequences.

if appUserID == "" {
return NotificationCredentialsRevokeResponse{}, fmt.Errorf("refusing to send a request with empty appUserID")
}

data, err := json.Marshal(NotificationCredentialsRevokeRequest{
AppUserID: appUserID,
PurgeSession: purgeSession,
})
if err != nil {
return NotificationCredentialsRevokeResponse{}, fmt.Errorf("marshaling the request data: %w", err)
}
req, err := request.NewRequestWithBearerToken(http.MethodPost, api.agent, api.baseURL, notificationTokenRevokeURL, "application/json", "", "gzip, deflate", bytes.NewBuffer(data), token)
if err != nil {
return NotificationCredentialsRevokeResponse{}, fmt.Errorf("creating nc credentials revoke request: %w", err)
}
rawResp, err := api.do(req)
if err != nil {
return NotificationCredentialsRevokeResponse{}, fmt.Errorf("executing HTTP POST request: %w", err)
}
defer rawResp.Body.Close()
out, err := io.ReadAll(rawResp.Body)
if err != nil {
return NotificationCredentialsRevokeResponse{}, fmt.Errorf("reading HTTP response body: %w", err)
}

var resp NotificationCredentialsRevokeResponse
if err := json.Unmarshal(out, &resp); err != nil {
return NotificationCredentialsRevokeResponse{}, fmt.Errorf("unmarshalling HTTP response: %w", err)
}
return resp, nil
}

func (api *DefaultAPI) Logout(token string) error {
resp, err := api.request(urlOAuth2Logout, http.MethodPost, nil, token)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions core/urls.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ const (
// notificationTokenURL defines url to retrieve Notification Center credentials
notificationTokenURL = "/v1/notifications/tokens"

// notificationTokenRevokeURL defines url to revoke Notification Center credentials
notificationTokenRevokeURL = "/v1/notifications/tokens/revoke"

// UsersURL defines url to create a new user
UsersURL = "/v1/users"

Expand Down
4 changes: 4 additions & 0 deletions daemon/rpc_connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ func (validCredentialsAPI) NotificationCredentials(token, appUserID string) (cor
return core.NotificationCredentialsResponse{}, nil
}

func (v validCredentialsAPI) NotificationCredentialsRevoke(token, appUserID string, purgeSession bool) (core.NotificationCredentialsRevokeResponse, error) {
return core.NotificationCredentialsRevokeResponse{}, nil
}

func (validCredentialsAPI) ServiceCredentials(string) (*core.CredentialsResponse, error) {
return &core.CredentialsResponse{
NordlynxPrivateKey: "nordpriv",
Expand Down
11 changes: 8 additions & 3 deletions daemon/rpc_logout.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,20 @@ func (r *RPC) Logout(ctx context.Context, in *pb.LogoutRequest) (*pb.Payload, er
return &pb.Payload{Type: internal.CodeFailure}, nil
}

if err := r.ncClient.Stop(); err != nil {
log.Println(internal.WarningPrefix, err)
}

tokenData, ok := cfg.TokensData[cfg.AutoConnectData.ID]
if !ok {
return &pb.Payload{Type: internal.CodeFailure}, nil
}

if !in.GetPersistToken() {
if !r.ncClient.Revoke(internal.IsDevEnv(string(r.environment))) {
log.Println(internal.WarningPrefix, "error revoking NC token")
}

if err := r.api.DeleteToken(tokenData.Token); err != nil {
log.Println(internal.ErrorPrefix, "deleting token: ", err)
switch {
Expand Down Expand Up @@ -87,9 +95,6 @@ func (r *RPC) Logout(ctx context.Context, in *pb.LogoutRequest) (*pb.Payload, er
return nil, err
}

if err := r.ncClient.Stop(); err != nil {
log.Println(internal.WarningPrefix, err)
}
r.publisher.Publish("user logged out")

// RenewToken being empty means user logged in using Access Token
Expand Down
6 changes: 4 additions & 2 deletions daemon/rpc_logout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"context"
"testing"

"github.com/stretchr/testify/assert"

"github.com/NordSecurity/nordvpn-linux/config"
"github.com/NordSecurity/nordvpn-linux/core"
"github.com/NordSecurity/nordvpn-linux/daemon/pb"
Expand All @@ -12,14 +14,14 @@ import (
"github.com/NordSecurity/nordvpn-linux/nc"
"github.com/NordSecurity/nordvpn-linux/test/mock/networker"
testnorduser "github.com/NordSecurity/nordvpn-linux/test/mock/norduser/service"
"github.com/stretchr/testify/assert"
)

type mockNC struct {
nc.NotificationClient
}

func (mockNC) Stop() error { return nil }
func (mockNC) Stop() error { return nil }
func (mockNC) Revoke(bool) bool { return true }

type mockApi struct {
core.CombinedAPI
Expand Down
13 changes: 10 additions & 3 deletions daemon/rpc_set_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ func (r *RPC) SetDefaults(ctx context.Context, in *pb.Empty) (*pb.Payload, error
log.Println(internal.WarningPrefix, err)
}

if err := r.ncClient.Stop(); err != nil {
log.Println(internal.WarningPrefix, err)
}

if internal.IsDevEnv(string(r.environment)) {
if !r.ncClient.Revoke(true) {
log.Println(internal.WarningPrefix, "error revoking token")
}
}

if err := r.cm.Reset(); err != nil {
log.Println(internal.ErrorPrefix, err)
return &pb.Payload{
Expand All @@ -48,9 +58,6 @@ func (r *RPC) SetDefaults(ctx context.Context, in *pb.Empty) (*pb.Payload, error

r.events.Settings.Defaults.Publish(nil)
r.events.Settings.Publish(cfg)
if err := r.ncClient.Stop(); err != nil {
log.Println(internal.WarningPrefix, err)
}

return &pb.Payload{
Type: internal.CodeSuccess,
Expand Down
29 changes: 28 additions & 1 deletion nc/creds_fetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package nc

import (
"fmt"
"strings"

"github.com/google/uuid"

"github.com/NordSecurity/nordvpn-linux/config"
"github.com/NordSecurity/nordvpn-linux/core"
"github.com/google/uuid"
)

type CredentialsGetter struct {
Expand Down Expand Up @@ -87,3 +89,28 @@ func (cf *CredentialsGetter) GetCredentialsFromAPI() (config.NCData, error) {
return c
})
}

// RevokeCredentials revokes credentials
func (cf *CredentialsGetter) RevokeCredentials(purgeSession bool) (bool, error) {
var cfg config.Config
if err := cf.cm.Load(&cfg); err != nil {
return false, fmt.Errorf("reading cfg: %w", err)
}
userID := cfg.AutoConnectData.ID
tokenData := cfg.TokensData[userID]
ncData := tokenData.NCData

if !areNCCredentialsValid(ncData) {
return false, ErrInvalidCredentials
}

resp, err := cf.api.NotificationCredentialsRevoke(tokenData.Token, tokenData.NCData.UserID.String(), purgeSession)
if err != nil {
return false, fmt.Errorf("error revoking token: %w", err)
}
if strings.ToLower(resp.Status) == "ok" {
return true, nil
} else {
return false, fmt.Errorf("response status: %s", resp.Status)
}
}
22 changes: 22 additions & 0 deletions nc/nc.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ type AcknowledgementPayload struct {
type NotificationClient interface {
Start() error
Stop() error
Revoke(bool) bool
}

type ClientBuilder interface {
Expand Down Expand Up @@ -474,3 +475,24 @@ func (c *Client) Stop() error {

return nil
}

// Revoke revokes the NC communication token
func (c *Client) Revoke(purgeSession bool) bool {
c.startMu.Lock()
defer c.startMu.Unlock()

if c.started {
log.Println(logPrefix, "attempt to revoke token for running client")
return false
}

log.Println(logPrefix, "revoking token, purgeSession:", purgeSession)
ok, err := c.credsFetcher.RevokeCredentials(purgeSession)
if ok {
log.Println(logPrefix, "token revoked successfully")
return true
} else {
log.Println(logPrefix, "token not revoked:", err)
return false
}
}
9 changes: 7 additions & 2 deletions test/mock/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,19 @@ package core
import "github.com/NordSecurity/nordvpn-linux/core"

type CredentialsAPIMock struct {
NotificationCredentialsResponse core.NotificationCredentialsResponse
NotificationCredentialsError error
NotificationCredentialsResponse core.NotificationCredentialsResponse
NotificationCredentialsRevokeResponse core.NotificationCredentialsRevokeResponse
NotificationCredentialsError error
}

func (c *CredentialsAPIMock) NotificationCredentials(token, appUserID string) (core.NotificationCredentialsResponse, error) {
return c.NotificationCredentialsResponse, c.NotificationCredentialsError
}

func (c *CredentialsAPIMock) NotificationCredentialsRevoke(token, appUserID string, purgeSession bool) (core.NotificationCredentialsRevokeResponse, error) {
return c.NotificationCredentialsRevokeResponse, nil
}

func (*CredentialsAPIMock) ServiceCredentials(string) (*core.CredentialsResponse, error) {
return nil, nil
}
Expand Down

0 comments on commit 89720eb

Please sign in to comment.