Headless SSO web endpoint and UI

[jakule]

Closes #22272

[ravicious]

Is "Headless" here supposed to be uppercase? It's inconsistent with this fragment:

teleport/lib/client/api.go

Lines 3462 to 3463 in eda29be

	fmt.Fprintf(tc.Stdout, "Complete headless authentication in your local web browser:\n\n%s\n"+
	"\nor execute this command in your local terminal:\n\n%s\n", webUILink, tshApprove)

[jakule]

Lowercase, fixed.

[ravicious]

Same q about capitalization as above.

[jakule]

The same a as above. It should be lowercase.

[ravicious]

Nit: I assume this catch is supposed to catch errors from auth.headlessSSOGet and we don't expect setIpAddress to throw errors. In cases like this, I find the double argument form of then to express the intent better. It's a bit unfortunate how Prettier formats it though:

    auth.headlessSSOGet(requestId).then(setIpAddress, e => {
      setState({
        ...state,
        status: 'error',
        errorText: e.toString(),
      });
    });

[ravicious]

Is there an easy way to test this feature locally?

[jakule]

Yes, checkout this branch, enable mandatory webauthn (this is fixed in https://github.com/gravitational/teleport/pull/23271/files), call tsh ls --headless --user=<user> --proxy=<proxy> and follow the prompts. If you have any questions, let me know.

[ravicious]

Thanks. I made a very silly assumption by thinking that because the component has Sso in its name, then I need to have SAML or OIDC enabled on my cluster. 🤦

Do you think something like HeadlessRequest would be a better name than HeadlessSso for this route?

[ravicious]

This says "click cancel" but the button to close the dialog says "Reject".

[jakule]

Good point, I'll rename that.

[ravicious]

If I reject the request and then refresh the URL, I see a spinner for a solid few seconds until eventually the deadline gets exceeded and I see "ApiError: rpc error: code = Unknown desc = context deadline exceeded" with a half functioning dialog (clicking Reject doesn't seem to do anything and Retry asks me for a hardware key touch).

Would it be possible to display the error sooner?

[jakule]

I changed the UI error screen. Let me know what do you think about it.

[ravicious]

Thanks. I made a very silly assumption by thinking that because the component has Sso in its name, then I need to have SAML or OIDC enabled on my cluster. 🤦

Do you think something like HeadlessRequest would be a better name than HeadlessSso for this route?

[ravicious]

I think it's good enough for the 12.2 release. I'd consider two improvements in the future:

• As I mentioned before, return the error immediately if the session is not valid. The user shouldn't have to wait for the deadline if the session was rejected already or if there's a typo in the UUID because the user copy-pasted just a part of the URL from the terminal.
• Accepting a request and then opening its URL again shows the modal again, as if I didn't approve it already. Only after clicking Approve or Reject I get a relevant error message. It feels like this should be shown immediately instead.

[ravicious]

I'm not a fan of splitting a component into parts like this, mostly because you can no longer read the component top to bottom to understand the structure of what is going to end up being rendered.

The conditionals could be inlined which IMHO would do a better job of showing what goes in the dialog content and what goes in the footer:

Updated version of HeadlessRequestDialog

export default function HeadlessRequestDialog({
  ipAddress,
  onAccept,
  onReject,
  errorText,
}: Props) {
  return (
    <Dialog dialogCss={() => ({ width: '400px' })} open={true}>
      <DialogHeader style={{ flexDirection: 'column' }}>
        <DialogTitle textAlign="center">
          Host {ipAddress} wants to execute a command
        </DialogTitle>
      </DialogHeader>
      <DialogContent mb={6}>
        {errorText && (
          <Danger mt={2} width="100%">
            {errorText}
          </Danger>
        )}
        <Text textAlign="center">
          {errorText ? (
            <>
              The requested session doesn't exist or is invalid. Please generate
              a new request.
              <br />
              <br />
              You can close this window.
            </>
          ) : (
            <>
              Someone has initiated a command from {ipAddress}. If it was not
              you, click reject and contact your administrator.
              <br />
              <br />
              If it was you, please use your hardware key to approve.
            </>
          )}
        </Text>
      </DialogContent>
      <DialogFooter textAlign="center">
        {!errorText && (
          <>
            <ButtonPrimary onClick={onAccept} autoFocus mr={3} width="130px">
              Approve
            </ButtonPrimary>
            <ButtonSecondary onClick={onReject}>Reject</ButtonSecondary>
          </>
        )}
      </DialogFooter>
    </Dialog>
  );
}

[jakule]

Not React Dev. Applying what people tells me is better 😅

[ravicious]

Suggested change

	click reject and contact your administrator.
	click Reject and contact your administrator.

From what I see this is the capitalization we use in docs. Grepping the project for "\bclick " didn't really return any results from the web folder.

[jakule]

Added

[jakule]

I think it's good enough for the 12.2 release. I'd consider two improvements in the future:

• As I mentioned before, return the error immediately if the session is not valid. The user shouldn't have to wait for the deadline if the session was rejected already or if there's a typo in the UUID because the user copy-pasted just a part of the URL from the terminal.
• Accepting a request and then opening its URL again shows the modal again, as if I didn't approve it already. Only after clicking Approve or Reject I get a relevant error message. It feels like this should be shown immediately instead.

@ravicious Thank you for the comments. I created an issue to track them. I just want to point out that this PR only introduces "REST" API endpoints and WebUI changes. The issues described above are more related to the underlying implementation which IMO should be fixed as a separate PR.