Skip to content

gRPC conversions - SSO Auth Connectors#13073

Merged
Joerger merged 23 commits into
masterfrom
joerger/grpc-conversions/sso-auth-requests
Jun 8, 2022
Merged

gRPC conversions - SSO Auth Connectors#13073
Joerger merged 23 commits into
masterfrom
joerger/grpc-conversions/sso-auth-requests

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Jun 1, 2022

Converts the following http endpoints to gRPC:

  • CreateOIDCAuthRequest
  • CreateSAMLAuthRequest
  • CreateGithubAuthRequest
  • GetOIDCAuthRequest
  • GetSAMLAuthRequest
  • GetGithubAuthRequest
  • GetSSODiagnosticInfo

Since the last 4 endpoints are brand new and are being released in v10, they do not need http fallback logic.

See #12783 (comment) for motivation behind this PR.

e PR - https://github.com/gravitational/teleport.e/pull/458

Updates #6394

@Joerger Joerger force-pushed the joerger/grpc-conversions/sso-auth-requests branch from dba3cb6 to eee64e6 Compare June 2, 2022 23:45
@Joerger Joerger marked this pull request as ready for review June 3, 2022 01:05
@github-actions github-actions Bot requested review from lxea and zmb3 June 3, 2022 01:06
@github-actions github-actions Bot added the tctl tctl - Teleport admin tool label Jun 3, 2022
@Joerger Joerger requested review from Tener and strideynet June 3, 2022 01:06
@Tener
Copy link
Copy Markdown
Contributor

Tener commented Jun 3, 2022

Thank you for this @Joerger, this set of changes is even larger than I expected it to be. It looks good though.

Some unit tests are failing, but this is easily fixable stuff:

--- FAIL: TestSSODiagnosticInfo (2.19s)
panic: invalid Go type *types.AssertionInfo for field types.SSODiagnosticInfo.SAMLAssertionInfo [recovered]
	panic: invalid Go type *types.AssertionInfo for field types.SSODiagnosticInfo.SAMLAssertionInfo

@Joerger Joerger force-pushed the joerger/grpc-conversions/sso-auth-requests branch from e4f6f8f to 3585a6e Compare June 3, 2022 17:42
@Joerger Joerger force-pushed the joerger/grpc-conversions/sso-auth-requests branch from 6f86540 to a2c6180 Compare June 3, 2022 18:57
Copy link
Copy Markdown
Contributor

@strideynet strideynet left a comment

Choose a reason for hiding this comment

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

Looks good just one concern.

Comment thread lib/auth/auth_with_roles.go Outdated
@Joerger Joerger enabled auto-merge (squash) June 6, 2022 18:34
@Joerger Joerger force-pushed the joerger/grpc-conversions/sso-auth-requests branch from d9c58ee to 91da847 Compare June 6, 2022 19:03
@Joerger Joerger disabled auto-merge June 6, 2022 19:25
@Joerger Joerger enabled auto-merge (squash) June 7, 2022 23:39
@Joerger Joerger merged commit 9cc58cc into master Jun 8, 2022
@rosstimothy
Copy link
Copy Markdown
Contributor

I think this breaks backward compatibility. I'm unable to login via GitHub when running v10 auth and v9.3.6 proxy.

2022-06-16T10:30:47-04:00 ERRO [WEB]       Error creating auth request. auth:github error:[
ERROR REPORT:
Original Error: *trace.BadParameterError request: json: cannot unmarshal number into Go value of type string
Stack Trace:

Caught:
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/httplib/httplib.go:155 github.com/gravitational/teleport/lib/httplib.ConvertResponse
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/auth/clt.go:278 github.com/gravitational/teleport/lib/auth.(*Client).PostJSON
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/auth/clt.go:1244 github.com/gravitational/teleport/lib/auth.(*Client).CreateGithubAuthRequest
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/web/apiserver.go:1063 github.com/gravitational/teleport/lib/web.(*Handler).githubLoginWeb
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/web/apiserver.go:2551 github.com/gravitational/teleport/lib/web.(*Handler).WithRedirect.func1
	/tmp/build-darwin-amd64/go/pkg/mod/github.com/gravitational/httprouter@v1.3.1-0.20220408074523-c876c5e705a5/router.go:399 github.com/julienschmidt/httprouter.(*Router).ServeHTTP
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/net/http/server.go:2090 net/http.StripPrefix.func1
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/net/http/server.go:2047 net/http.HandlerFunc.ServeHTTP
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/web/apiserver.go:454 github.com/gravitational/teleport/lib/web.NewHandler.func1
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/net/http/server.go:2047 net/http.HandlerFunc.ServeHTTP
	/tmp/build-darwin-amd64/go/pkg/mod/github.com/gravitational/httprouter@v1.3.1-0.20220408074523-c876c5e705a5/router.go:460 github.com/julienschmidt/httprouter.(*Router).ServeHTTP
	/private/tmp/build-darwin-amd64/go/src/github.com/gravitational/teleport/lib/web/apiserver.go:215 github.com/gravitational/teleport/lib/web.(*APIHandler).ServeHTTP
	/tmp/build-darwin-amd64/go/pkg/mod/github.com/gravitational/oxy@v0.0.0-20211213172937-a1ba0900a4c9/ratelimit/tokenlimiter.go:118 github.com/gravitational/oxy/ratelimit.(*TokenLimiter).ServeHTTP
	/tmp/build-darwin-amd64/go/pkg/mod/github.com/gravitational/oxy@v0.0.0-20211213172937-a1ba0900a4c9/connlimit/connlimit.go:75 github.com/gravitational/oxy/connlimit.(*ConnLimiter).ServeHTTP
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/net/http/server.go:2879 net/http.serverHandler.ServeHTTP
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/net/http/server.go:3480 net/http.initALPNRequest.ServeHTTP
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/net/http/h2_bundle.go:5849 net/http.(*http2serverConn).runHandler
	/var/folders/ys/8czjjsys38x504kj8172pd_m0000gp/T/drone-MH8sayoNWmwkJaEN/home/drone/build-12784-1655249166-toolchains/go/src/runtime/asm_amd64.s:1581 runtime.goexit
User Message: request: json: cannot unmarshal number into Go value of type string
] web/apiserver.go:1071

This looks to be caused by casting the CertTTL to a types.Duration:

teleport/api/types/types.proto

Lines 2714 to 2715 in 78c1450

// CertTTL is the TTL of the certificate user wants to get
int64 CertTTL = 8 [ (gogoproto.jsontag) = "cert_ttl", (gogoproto.casttype) = "Duration" ];

The custom marshalling/unmarshalling is expecting to be working with a string which is no longer the case

func (d *Duration) UnmarshalJSON(data []byte) error {
if len(data) == 0 {
return nil
}
var stringVar string
if err := json.Unmarshal(data, &stringVar); err != nil {
return trace.Wrap(err)
}
if stringVar == constants.DurationNever {
*d = Duration(0)
return nil
}
out, err := parseDuration(stringVar)
if err != nil {
return trace.BadParameter(err.Error())
}
*d = out
return nil
}

Joerger added a commit that referenced this pull request Oct 6, 2022
* Complete deprecation for OIDC RedirectURL - #12054.

* Complete deprecation for CreateSessionTracker - #12304.

* Complete deprecation for SSO Auth Request http endpoints - #13073.

* Complete deprecation for #12795.

* Complete deprecation for http GenerateToken - #9024.
Joerger added a commit that referenced this pull request Oct 6, 2022
* Complete deprecation for OIDC RedirectURL - #12054.

* Complete deprecation for CreateSessionTracker - #12304.

* Complete deprecation for SSO Auth Request http endpoints - #13073.

* Complete deprecation for #12795.

* Complete deprecation for http GenerateToken - #9024.
Joerger added a commit that referenced this pull request Oct 6, 2022
* Complete deprecation for OIDC RedirectURL - #12054.

* Complete deprecation for CreateSessionTracker - #12304.

* Complete deprecation for SSO Auth Request http endpoints - #13073.

* Complete deprecation for #12795.

* Complete deprecation for http GenerateToken - #9024.
@Joerger Joerger deleted the joerger/grpc-conversions/sso-auth-requests branch March 20, 2023 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tctl tctl - Teleport admin tool

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants