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

Broken on Windows #20

Open
silenuz opened this issue Dec 20, 2020 · 5 comments
Open

Broken on Windows #20

silenuz opened this issue Dec 20, 2020 · 5 comments

Comments

@silenuz
Copy link

silenuz commented Dec 20, 2020

While version 1.0 didn't allow saving to the local keychain in windows, which appears to be fixed in 1.1, however while it's nice that they are actually saved now it is impossible to retrieve them.

xoauth connect test
Requesting OIDC metadata from https://identity.xero.com/.well-known/openid-configuration
Received OIDC metadata for authority: https://identity.xero.com
Opening browser window
Received OIDC response
Exchanging code at token endpoint: https://identity.xero.com/connect/token
Validating token
Using public key: 1CAF8E66772D6DC028D6726FD0261581570EFC19
Storing tokens in local keychain
{
    "access_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFDQUY4RT-SHORTENED",
    "id_token": "eyJhbGciOiJSUzI1NiIsImtpZCI6IjFDQUY4RTY2Nz-SHORTENED",
    "refresh_token": "9ab45fa105b8d25999685483135ec6e9b4363fc6ed3742b6a3b481cec211b364",
    "token_type": "Bearer",
    "expires_in": 1800,
    "expires_at": 1608487433
}

a call to xoauth token client results in:

C:\xoauth>xoauth token test
panic: runtime error: invalid memory address or nil pointer dereference
[signal 0xc0000005 code=0x0 addr=0x18 pc=0x7a7b6d]

goroutine 1 [running]:
github.com/XeroAPI/xoauth/pkg/keyring.WindowsKeyRingService.GetTokens(0xc00001e0b0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/runner/work/xoauth/xoauth/pkg/keyring/windowsKeyring.go:63 +0x1dd
github.com/XeroAPI/xoauth/pkg/db.(*CredentialStore).GetTokens(0xc0000411f0, 0xc00001e0b0, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/runner/work/xoauth/xoauth/pkg/db/db.go:283 +0xa7
github.com/XeroAPI/xoauth/pkg/tokens.ShowTokens(0xc0000411f0, 0xc00001e0b0, 0x4, 0x800000)
        /home/runner/work/xoauth/xoauth/pkg/tokens/tokens.go:22 +0xee
github.com/XeroAPI/xoauth/cmd.init.0.func11(0xc0000f42c0, 0xc0000411d0, 0x1, 0x1)
        /home/runner/work/xoauth/xoauth/cmd/root.go:189 +0x133
github.com/spf13/cobra.(*Command).execute(0xc0000f42c0, 0xc0000411a0, 0x1, 0x1, 0xc0000f42c0, 0xc0000411a0)
        /home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:846 +0x2b1
github.com/spf13/cobra.(*Command).ExecuteC(0xcaf140, 0x0, 0x850660, 0xc000024118)
        /home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:950 +0x350
github.com/spf13/cobra.(*Command).Execute(...)
        /home/runner/go/pkg/mod/github.com/spf13/[email protected]/command.go:887
github.com/XeroAPI/xoauth/cmd.Execute(...)
        /home/runner/work/xoauth/xoauth/cmd/root.go:254
main.main()
        /home/runner/work/xoauth/xoauth/xoauth.go:13 +0x4e

Which in turns breaks the --refresh option as well. It would be nice to have a working version on windows.

Update:
I don't believe it is actually saving the tokens in the keychain. If one deletes a connection, this is the output:

? Are you sure you want to delete this connection? Yes
No tokens to delete for test
Connection deleted

Found the problem. It's in the windowskeyring.go file. Inthe following function
func (service WindowsKeyRingService) GetTokens(item string) (oidc.TokenResultSet, error)

On line 63:

// Since these params are optional, ignore not found errors
	if err.Error() != keyring.ErrNotFound.Error() {
		return result, err
	}

At this point err is nil. So maybe it should be something like:

// Since these params are optional, ignore not found errors
	if err != nil && err.Error() != keyring.ErrNotFound.Error() {
		return result, err
	}

This fixed problems for me, but unfortunately I am unfamiliar with GO and have to now figure out how to recompile.

@chrisgleaves
Copy link

I have exactly the same issue (running W10 20H2)

@AjitSharmaTofrum
Copy link

@silenuz were you are to resolve the issue. If yes, can you please share the steps.

@silenuz
Copy link
Author

silenuz commented Jan 20, 2021

Line 63 in the windowskeyring.go file is where it breaks.

On line 63:

// Since these params are optional, ignore not found errors
	if err.Error() != keyring.ErrNotFound.Error() {
		return result, err
	}

At this point err is nil. So maybe it should be:

// Since these params are optional, ignore not found errors
	if err != nil && err.Error() != keyring.ErrNotFound.Error() {
		return result, err
	}

The code is in Go, so it's easy enough to fix and recompile. As this project seems abandoned if there is interest I may consider forking this project and fixing it.

@manishchhettri
Copy link

This was a real pain to work around, works fine on mac fails in windows. Worked past it by doing the setup and the connect via powershell command line invoked through a java program. The refresh code was done in java using com.google.api.client.auth.oauth2 APIs to refresh the token.

@dm-efi
Copy link

dm-efi commented Jul 19, 2021

I suspect the bug occurs when the key storage requirements doesn't need a second 2 windows key ring because the number of scopes is small enough to fit into one key ring. When it tries to read back the second key ring it crashes. I'd rather Xero fix this and create another executable rather than download and install the go environment myself.

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

No branches or pull requests

5 participants