XeroAPI / xoauth

A CLI tool for obtaining JWTs from OpenId Connect providers
MIT License
49 stars 16 forks source link

Broken on Windows #20

Open silenuz opened 3 years ago

silenuz commented 3 years ago

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/cobra@v1.0.0/command.go:846 +0x2b1
github.com/spf13/cobra.(*Command).ExecuteC(0xcaf140, 0x0, 0x850660, 0xc000024118)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.0.0/command.go:950 +0x350
github.com/spf13/cobra.(*Command).Execute(...)
        /home/runner/go/pkg/mod/github.com/spf13/cobra@v1.0.0/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 commented 3 years ago

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

AjitSharmaTofrum commented 3 years ago

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

silenuz commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.