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

Use token source instead of non-refreshable token #151

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 25 additions & 18 deletions pkg/account/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ import (
"github.com/teslamotors/vehicle-command/pkg/connector"
"github.com/teslamotors/vehicle-command/pkg/connector/inet"
"github.com/teslamotors/vehicle-command/pkg/vehicle"
"golang.org/x/oauth2"
)

var (
//go:embed version.txt
libraryVersion string
)
//go:embed version.txt
var libraryVersion string

func buildUserAgent(app string) string {
library := strings.TrimSpace("tesla-sdk/" + libraryVersion)
Expand Down Expand Up @@ -64,10 +63,9 @@ func buildUserAgent(app string) string {
// Account allows interaction with a Tesla account.
type Account struct {
// The default UserAgent is constructed from the global UserAgent, but can be overridden.
UserAgent string
authHeader string
Host string
client http.Client
UserAgent string
Host string
client *http.Client
}

// We don't parse JWTs beyond what's required to extract the API server domain name
Expand All @@ -76,8 +74,10 @@ type oauthPayload struct {
OUCode string `json:"ou_code"`
}

var domainRegEx = regexp.MustCompile(`^[A-Za-z0-9-.]+$`) // We're mostly interested in stopping paths; the http package handles the rest.
var remappedDomains = map[string]string{} // For use during development; populate in an init() function.
var (
domainRegEx = regexp.MustCompile(`^[A-Za-z0-9-.]+$`) // We're mostly interested in stopping paths; the http package handles the rest.
remappedDomains = map[string]string{} // For use during development; populate in an init() function.
)

const defaultDomain = "fleet-api.prd.na.vn.cloud.tesla.com"

Expand Down Expand Up @@ -113,8 +113,11 @@ func (p *oauthPayload) domain() string {
}

// New returns an [Account] that can be used to fetch a [vehicle.Vehicle].
andig marked this conversation as resolved.
Show resolved Hide resolved
// Optional userAgent can be passed in - otherwise it will be generated from code
func New(oauthToken, userAgent string) (*Account, error) {
func New(ts oauth2.TokenSource, userAgent string) (*Account, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently tokens are invalidated immediately after another refresh token get issued. I'm afraid that building this functionality is going to create downstream errors for folks who manages their tokens cross app/manually

Copy link
Author

@andig andig Feb 3, 2024

Choose a reason for hiding this comment

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

To be fair, this PR is from before that change. Actually, I think this makes this PR even more important. How you build your token source is up to you.

oauthToken, err := ts.Token()
if err != nil {
return nil, err
}
parts := strings.Split(oauthToken, ".")
andig marked this conversation as resolved.
Show resolved Hide resolved
if len(parts) != 3 {
return nil, fmt.Errorf("client provided malformed OAuth token")
Expand All @@ -132,10 +135,15 @@ func New(oauthToken, userAgent string) (*Account, error) {
if domain == "" {
return nil, fmt.Errorf("client provided OAuth token with invalid audiences")
}
client := http.DefaultClient
client.Transport = &oauth2.Transport{
Source: ts,
Base: http.DefaultClient.Transport,
}
return &Account{
UserAgent: buildUserAgent(userAgent),
authHeader: "Bearer " + strings.TrimSpace(oauthToken),
Host: domain,
UserAgent: buildUserAgent(userAgent),
client: client,
Host: domain,
}, nil
}

Expand All @@ -147,7 +155,7 @@ func New(oauthToken, userAgent string) (*Account, error) {
// sessions parameter may also be nil, but providing a cache.SessionCache avoids a round-trip
// handshake with the Vehicle in subsequent connections.
func (a *Account) GetVehicle(ctx context.Context, vin string, privateKey authentication.ECDHPrivateKey, sessions *cache.SessionCache) (*vehicle.Vehicle, error) {
conn := inet.NewConnection(vin, a.authHeader, a.Host, a.UserAgent)
conn := inet.NewConnection(vin, a.client, a.Host, a.UserAgent)
car, err := vehicle.NewVehicle(conn, privateKey, sessions)
if err != nil {
conn.Close()
Expand All @@ -168,7 +176,6 @@ func (a *Account) Get(ctx context.Context, endpoint string) ([]byte, error) {
log.Debug("Requesting %s...", url)
request.Header.Set("Content-Type", "application/json")
request.Header.Set("User-Agent", a.UserAgent)
request.Header.Set("Authorization", a.authHeader)
response, err := a.client.Do(request)
if err != nil {
return nil, fmt.Errorf("error fetching %s: %w", endpoint, err)
Expand All @@ -188,7 +195,7 @@ func (a *Account) Get(ctx context.Context, endpoint string) ([]byte, error) {
}

func (a *Account) sendFleetAPICommand(ctx context.Context, endpoint string, command interface{}) ([]byte, error) {
return inet.SendFleetAPICommand(ctx, &a.client, a.UserAgent, a.authHeader, fmt.Sprintf("https://%s/%s", a.Host, endpoint), command)
return inet.SendFleetAPICommand(ctx, a.client, a.UserAgent, fmt.Sprintf("https://%s/%s", a.Host, endpoint), command)
}

// Post sends an HTTP POST request to endpoint.
Expand Down
21 changes: 13 additions & 8 deletions pkg/account/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,42 @@ import (
"encoding/json"
"fmt"
"testing"

"golang.org/x/oauth2"
)

func b64Encode(payload string) string {
return base64.RawStdEncoding.EncodeToString([]byte(payload))
}

func TestNewAccount(t *testing.T) {
ts := func(token string) oauth2.TokenSource {
return oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
}
validDomain := "fleet-api.example.tesla.com"
if _, err := New("", ""); err == nil {
if _, err := New(ts(""), ""); err == nil {
t.Error("Returned success empty JWT")
}
if _, err := New(b64Encode(validDomain), ""); err == nil {
if _, err := New(ts(b64Encode(validDomain)), ""); err == nil {
t.Error("Returned success on one-field JWT")
}
if _, err := New("x."+b64Encode(validDomain), ""); err == nil {
if _, err := New(ts("x."+b64Encode(validDomain)), ""); err == nil {
t.Error("Returned success on two-field JWT")
}
if _, err := New("x."+b64Encode(validDomain)+"y.z", ""); err == nil {
if _, err := New(ts("x."+b64Encode(validDomain)+"y.z"), ""); err == nil {
t.Error("Returned success on four-field JWT")
}
if _, err := New("x."+validDomain+".y", ""); err == nil {
if _, err := New(ts("x."+validDomain+".y"), ""); err == nil {
t.Error("Returned success on non-base64 encoded JWT")
}
if _, err := New("x."+b64Encode("{\"aud\": \"example.com\"}")+".y", ""); err == nil {
if _, err := New(ts("x."+b64Encode("{\"aud\": \"example.com\"}")+".y"), ""); err == nil {
t.Error("Returned success on untrusted domain")
}
if _, err := New("x."+b64Encode(fmt.Sprintf("{\"aud\": \"%s\"}", validDomain))+".y", ""); err == nil {
if _, err := New(ts("x."+b64Encode(fmt.Sprintf("{\"aud\": \"%s\"}", validDomain))+".y"), ""); err == nil {
t.Error("Returned when aud field not a list")
}

acct, err := New("x."+b64Encode(fmt.Sprintf("{\"aud\": [\"%s\"]}", validDomain))+".y", "")
acct, err := New(ts("x."+b64Encode(fmt.Sprintf("{\"aud\": [\"%s\"]}", validDomain))+".y"), "")
if err != nil {
t.Fatalf("Returned error on valid JWT: %s", err)
}
Expand Down
29 changes: 13 additions & 16 deletions pkg/connector/inet/inet.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (e *HttpError) Temporary() bool {
e.Code == http.StatusTooManyRequests
}

func SendFleetAPICommand(ctx context.Context, client *http.Client, userAgent, authHeader string, url string, command interface{}) ([]byte, error) {
func SendFleetAPICommand(ctx context.Context, client *http.Client, userAgent, url string, command interface{}) ([]byte, error) {
var body []byte
var ok bool
if body, ok = command.([]byte); !ok {
Expand All @@ -96,7 +96,6 @@ func SendFleetAPICommand(ctx context.Context, client *http.Client, userAgent, au

request.Header.Set("User-Agent", userAgent)
request.Header.Set("Content-type", "application/json")
request.Header.Set("Authorization", authHeader)
request.Header.Set("Accept", "*/*")

result, err := client.Do(request)
Expand Down Expand Up @@ -139,7 +138,7 @@ func ValidTeslaDomainSuffix(domain string) bool {
// response body is not necessarily nil if the error is set.
func (c *Connection) SendFleetAPICommand(ctx context.Context, endpoint string, command interface{}) ([]byte, error) {
url := fmt.Sprintf("https://%s/%s", c.serverURL, endpoint)
rsp, err := SendFleetAPICommand(ctx, &c.client, c.UserAgent, c.authHeader, url, command)
rsp, err := SendFleetAPICommand(ctx, c.client, c.UserAgent, url, command)
if err != nil {
var httpErr *HttpError
if errors.As(err, &httpErr) && httpErr.Code == http.StatusMisdirectedRequest {
Expand All @@ -155,26 +154,24 @@ func (c *Connection) SendFleetAPICommand(ctx context.Context, endpoint string, c

// Connection implements the connector.Connector interface by POSTing commands to a server.
type Connection struct {
UserAgent string
vin string
client http.Client
serverURL string
inbox chan []byte
authHeader string
UserAgent string
vin string
client *http.Client
serverURL string
inbox chan []byte

wakeLock sync.Mutex
lastPoke time.Time
}

// NewConnection creates a Connection.
func NewConnection(vin string, authHeader, serverURL, userAgent string) *Connection {
func NewConnection(vin string, client *http.Client, serverURL, userAgent string) *Connection {
Copy link
Collaborator

@sethterashima sethterashima Feb 13, 2024

Choose a reason for hiding this comment

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

The requirements for client need to be clearly documented, ideally with an example of setting up a token source. Alternatively, we could create a NewConnectionWithClient(client *http.Client, vin string ...) *Connection function (with documentation + example).

This could be mirrored in a account.NewWithClient(...) function.

conn := Connection{
UserAgent: userAgent,
vin: vin,
client: http.Client{},
serverURL: serverURL,
authHeader: authHeader,
inbox: make(chan []byte, connector.BufferSize),
UserAgent: userAgent,
vin: vin,
client: client,
serverURL: serverURL,
inbox: make(chan []byte, connector.BufferSize),
}
return &conn
}
Expand Down
4 changes: 3 additions & 1 deletion pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/teslamotors/vehicle-command/pkg/connector/inet"
"github.com/teslamotors/vehicle-command/pkg/protocol"
"github.com/teslamotors/vehicle-command/pkg/vehicle"
"golang.org/x/oauth2"
)

const (
Expand All @@ -33,7 +34,8 @@ func getAccount(req *http.Request) (*account.Account, error) {
if !ok {
return nil, fmt.Errorf("client did not provide an OAuth token")
}
return account.New(token, proxyProtocolVersion)
ts := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token})
return account.New(ts, proxyProtocolVersion)
}

// Proxy exposes an HTTP API for sending vehicle commands.
Expand Down