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

Fix initial token loading #10

Closed
wants to merge 1 commit into from

Conversation

Yohan460
Copy link
Contributor

@Yohan460 Yohan460 commented Oct 7, 2022

Currently token bootstrapping with the file backend is non-functional. This PR resolves that issue

When performing the token upload operation you'd get one of the two errors

{
  "error": "retrieving auth tokens: open db/nanomdm-dev.tokens.json: no such file or directory"
}

or

{
  "error": "mismatched consumer key"
}

This PR adds some error validation, and default value checking to resolve this issue

@jessepeterson
Copy link
Member

This is a good catch, not sure how I missed this. Obviously we can't err on retrieving the tokens first because it's a proper error if they're not yet set/exist yet. I think the correct fix is something like this (i.e. eat the error):

diff --git a/http/api/ckcheck.go b/http/api/ckcheck.go
index 5288c00..1ee4606 100644
--- a/http/api/ckcheck.go
+++ b/http/api/ckcheck.go
@@ -3,7 +3,6 @@ package api
 import (
        "context"
        "errors"
-       "fmt"
 
        "github.com/micromdm/nanodep/client"
 )
@@ -32,10 +31,7 @@ func NewCKCheck(store AuthTokensStore) *CKCheck {
 // the provided auth tokens.
 func (t *CKCheck) StoreAuthTokens(ctx context.Context, name string, tokens *client.OAuth1Tokens) error {
        prevTokens, err := t.AuthTokensRetriever.RetrieveAuthTokens(ctx, name)
-       if err != nil {
-               return fmt.Errorf("retrieving auth tokens: %w", err)
-       }
-       if prevTokens.ConsumerKey != tokens.ConsumerKey {
+       if err == nil && prevTokens != nil && prevTokens.ConsumerKey != tokens.ConsumerKey {
                return CKMismatch
        }
        return t.AuthTokensStorer.StoreAuthTokens(ctx, name, tokens)

The longer term fix, however, since I've discovered some cases where the Consumer Key can legitimately change is probably to add capability for a "force" mode which will override the tokens even if the CK changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants