influxdata / chronograf

Open source monitoring and visualization UI for the TICK stack
https://www.influxdata.com/time-series-platform/chronograf/
Other
1.51k stars 258 forks source link

oauth validation broken #5473

Closed benschweizer closed 4 years ago

benschweizer commented 4 years ago

PR#5458 introduced some oauth sanity checks which result in a failure:

ERRO[0000] Failed to validate Oauth settings: missing Google oauth setting[s]: client id, client secret  basepath=invalid component=server

I'm using GenericOAuth2 instead of GoogleOAuth, but both use the variable PublicURL. Now, with the checks of PR#5458 (https://github.com/influxdata/chronograf/pull/5458/files#diff-91bbeda7eb98a7adc57b9e47e2cf5c2bR167), the GoogleOAuth providers gets activated and fails because of missing parameters:

    if s.TokenSecret != "" && s.GoogleClientID != "" && s.GoogleClientSecret != "" && s.PublicURL != "" {
        return nil
    } else if s.GoogleClientID == "" && s.GoogleClientSecret == "" && s.PublicURL == "" {
        return errNoAuth
    }

The check for PublicURL should probably be removed?

glinton commented 4 years ago

@benschweizer can you post your obfuscated configuration and the full output of that log.

The check for PublicURL needs to remain in the google oauth check because that is required by google's oauth.

glinton commented 4 years ago

Was able to reproduce, adding fix.

nickidw commented 4 years ago

Are there any plans to push a new build for this, I'm sitting with a broken chronograf after upgrading to 1.8.4 seemingly because of this issue? I'm using the Generic OAuth provider with AzureAD.

glinton commented 4 years ago

@nickidw I've reached out to learn more about the 1.8.5 release that would fix that issue. In the meantime, would it be possible to use the Chronograf nightly build that also contains the fix for that?

nickidw commented 4 years ago

I'm ok at the moment thanks, it's working on 1.7.17. If I experience errors I'll think about going to nightly build.