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

feat: Password-less login with WebAuthn #195

Merged
merged 19 commits into from
Mar 25, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion .github/workflows/oci-dist-spec-content-discovery.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ name: OCI Distribution Spec

on:
workflow_call:
inputs:
debug_enabled:
type: boolean
description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
required: false
default: false

concurrency:
group: content-discovery-${{ github.workflow }}-${{ github.head_ref || github.run_id }}
Expand Down Expand Up @@ -81,7 +87,7 @@ jobs:
OCI_DEBUG: 0
- name: Setup tmate session if mode is debug and OpenRegistry or OCI Tests Fail
uses: mxschmitt/action-tmate@v3
if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
if: ${{ always() && (github.event_name == 'workflow_dispatch') && inputs.debug_enabled }}
- name: Set output report name
id: vars
run: echo "short_commit_hash=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
Expand Down
8 changes: 7 additions & 1 deletion .github/workflows/oci-dist-spec-content-management.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ name: OCI Distribution Spec

on:
workflow_call:
inputs:
debug_enabled:
type: boolean
description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
required: false
default: false

concurrency:
group: content-management-${{ github.workflow }}-${{ github.head_ref || github.run_id }}
Expand Down Expand Up @@ -81,7 +87,7 @@ jobs:
OCI_DEBUG: 0
- name: Setup tmate session if mode is debug and OpenRegistry or OCI Tests Fail
uses: mxschmitt/action-tmate@v3
if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
if: ${{ always() && (github.event_name == 'workflow_dispatch') && inputs.debug_enabled }}
- name: Set output report name
id: vars
run: echo "short_commit_hash=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/oci-dist-spec-pull.yml
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ jobs:
OCI_DEBUG: 0
- name: Setup tmate session if mode is debug and OpenRegistry or OCI Tests Fail
uses: mxschmitt/action-tmate@v3
if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
if: ${{ always() && (github.event_name == 'workflow_dispatch') && inputs.debug_enabled }}
- name: Set output report name
id: vars
run: echo "short_commit_hash=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
Expand Down
8 changes: 7 additions & 1 deletion .github/workflows/oci-dist-spec-push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@ name: OCI Distribution Spec

on:
workflow_call:
inputs:
debug_enabled:
type: boolean
description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
required: false
default: false

concurrency:
group: push-${{ github.workflow }}-${{ github.head_ref || github.run_id }}
Expand Down Expand Up @@ -82,7 +88,7 @@ jobs:
OCI_DEBUG: 0
- name: Setup tmate session if mode is debug and OpenRegistry or OCI Tests Fail
uses: mxschmitt/action-tmate@v3
if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
if: ${{ always() && (github.event_name == 'workflow_dispatch') && inputs.debug_enabled }}
- name: Set output report name
id: vars
run: echo "short_commit_hash=$(git rev-parse --short HEAD)" >> $GITHUB_OUTPUT
Expand Down
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@ certs
*.pem
*.bak
*.backup
config.yaml.bak
config.yml.bak
*.pem
7 changes: 3 additions & 4 deletions auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ func New(
pgStore postgres.PersistentStore,
logger telemetry.Logger,
) Authentication {

githubOAuth := &oauth2.Config{
ClientID: c.OAuth.Github.ClientID,
ClientSecret: c.OAuth.Github.ClientSecret,
Expand All @@ -50,7 +49,7 @@ func New(
}

ghClient := gh.NewClient(nil)
emailClient := email.New(&c.Email, c.WebAppEndpoint)
emailClient := email.New(&c.Email, c.WebAppConfig.Endpoint)

a := &auth{
c: c,
Expand All @@ -62,7 +61,7 @@ func New(
emailClient: emailClient,
}

go a.StateTokenCleanup()
go a.stateTokenCleanup()

return a
}
Expand All @@ -80,7 +79,7 @@ type (
)

// @TODO (jay-dee7) maybe a better way to do it?
func (a *auth) StateTokenCleanup() {
func (a *auth) stateTokenCleanup() {
// tick every 10 minutes, delete ant oauth state tokens which are older than 10 mins
guacamole marked this conversation as resolved.
Show resolved Hide resolved
// duration = 10mins, because github short lived code is valid for 10 mins
for range time.Tick(time.Second * 10) {
Expand Down
201 changes: 135 additions & 66 deletions auth/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,18 @@ package auth

import (
"context"
"errors"
"fmt"
"net/http"
"net/url"
"time"

"github.com/containerish/OpenRegistry/config"
"github.com/containerish/OpenRegistry/types"
"github.com/google/uuid"
"github.com/jackc/pgconn"
"github.com/jackc/pgerrcode"
"github.com/jackc/pgx/v4"
"github.com/labstack/echo/v4"
"golang.org/x/oauth2"
)
Expand All @@ -24,6 +29,7 @@ func (a *auth) LoginWithGithub(ctx echo.Context) error {
a.logger.Log(ctx, err)
return echoErr
}

a.oauthStateStore[state.String()] = time.Now().Add(time.Minute * 10)
url := a.github.AuthCodeURL(state.String(), oauth2.AccessTypeOffline)
a.logger.Log(ctx, nil)
Expand All @@ -36,11 +42,9 @@ func (a *auth) GithubLoginCallbackHandler(ctx echo.Context) error {
stateToken := ctx.FormValue("state")
_, ok := a.oauthStateStore[stateToken]
if !ok {
err := fmt.Errorf("INVALID_STATE_TOKEN")
echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
"error": err.Error(),
"message": "missing or invalid state token",
})
err := fmt.Errorf("missing or invalid state token")
uri := a.getGitHubErrorURI(http.StatusBadRequest, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}
Expand All @@ -52,22 +56,16 @@ func (a *auth) GithubLoginCallbackHandler(ctx echo.Context) error {
code := ctx.FormValue("code")
token, err := a.github.Exchange(context.Background(), code)
if err != nil {
echoErr := ctx.JSON(http.StatusBadRequest, echo.Map{
"error": err.Error(),
"message": "github exchange error",
"code": "GITHUB_EXCHANGE_ERR",
})
uri := a.getGitHubErrorURI(http.StatusBadRequest, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}

req, err := a.ghClient.NewRequest(http.MethodGet, "/user", nil)
if err != nil {
echoErr := ctx.JSON(http.StatusPreconditionFailed, echo.Map{
"error": err.Error(),
"message": "github client request failed",
"code": "GH_CLIENT_REQ_FAILED",
})
uri := a.getGitHubErrorURI(http.StatusBadRequest, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}
Expand All @@ -76,71 +74,76 @@ func (a *auth) GithubLoginCallbackHandler(ctx echo.Context) error {
var oauthUser types.User
_, err = a.ghClient.Do(ctx.Request().Context(), req, &oauthUser)
if err != nil {
echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
"error": err.Error(),
"message": "github client request execution failed",
"code": "GH_CLIENT_REQ_EXEC_FAILED",
})
uri := a.getGitHubErrorURI(http.StatusBadRequest, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}

oauthUser.Username = oauthUser.Login
id, err := uuid.NewRandom()
user, err := a.pgStore.GetOAuthUser(ctx.Request().Context(), oauthUser.Login, nil)
if err != nil {
echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
"error": err.Error(),
"message": "error creating oauth user id",
})
a.logger.Log(ctx, err)
return echoErr
}
oauthUser.Id = id.String()
err = a.storeGitHubUserIfDoesntExist(ctx.Request().Context(), err, &oauthUser)
if err != nil {
uri := a.getGitHubErrorURI(http.StatusConflict, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}
if err = a.finishGitHubCallback(ctx, oauthUser.Username, oauthUser.Id, token); err != nil {
uri := a.getGitHubErrorURI(http.StatusConflict, err.Error())
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, uri)
a.logger.Log(ctx, err)
return echoErr
}

accessToken, refreshToken, err := a.SignOAuthToken(oauthUser.Id, token)
if err != nil {
echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
"error": err.Error(),
"cause": "JWT_SIGNING",
})
a.logger.Log(ctx, err)
return echoErr
err = ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppConfig.RedirectURL)
a.logger.Log(ctx, nil)
return err
}

oauthUser.Password = refreshToken
if err = a.pgStore.AddOAuthUser(ctx.Request().Context(), &oauthUser); err != nil {
redirectPath := fmt.Sprintf("%s%s?error=%s", a.c.WebAppEndpoint, a.c.WebAppErrorRedirectPath, err.Error())
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, redirectPath)
if user.WebauthnConnected && !user.GithubConnected {
err = fmt.Errorf("username/email already exists")
uri := a.getGitHubErrorURI(http.StatusConflict, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}

sessionId, err := uuid.NewRandom()
if err != nil {
echoErr := ctx.JSON(http.StatusInternalServerError, echo.Map{
"error": err.Error(),
"cause": "error creating session id",
})
a.logger.Log(ctx, err)
return echoErr
if user.GithubConnected {
err = a.pgStore.UpdateOAuthUser(
ctx.Request().Context(),
oauthUser.Email,
oauthUser.Login,
oauthUser.NodeID,
nil,
)
if err != nil {
uri := a.getGitHubErrorURI(http.StatusConflict, err.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err)
return echoErr
}

if err = a.finishGitHubCallback(ctx, oauthUser.Login, user.Id, token); err != nil {
uri := a.getGitHubErrorURI(http.StatusConflict, err.Error())
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, uri)
a.logger.Log(ctx, err)
return echoErr
}

err = ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppConfig.RedirectURL)
a.logger.Log(ctx, nil)
return err
}
err = a.pgStore.AddSession(ctx.Request().Context(), sessionId.String(), refreshToken, oauthUser.Username)
if err != nil {
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppErrorRedirectPath)

// this will set the add session object to database and attaches cookies to the echo.Context object
if err = a.finishGitHubCallback(ctx, oauthUser.Username, oauthUser.Id, token); err != nil {
uri := a.getGitHubErrorURI(http.StatusConflict, err.Error())
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, uri)
a.logger.Log(ctx, err)
return echoErr
}
val := fmt.Sprintf("%s:%s", sessionId, oauthUser.Id)

sessionCookie := a.createCookie("session_id", val, false, time.Now().Add(time.Hour*750))
accessCookie := a.createCookie("access", accessToken, true, time.Now().Add(time.Hour*750))
refreshCookie := a.createCookie("refresh", refreshToken, true, time.Now().Add(time.Hour*750))

ctx.SetCookie(accessCookie)
ctx.SetCookie(refreshCookie)
ctx.SetCookie(sessionCookie)

err = ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppRedirectURL)
err = ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppConfig.RedirectURL)
a.logger.Log(ctx, nil)
return err
}
Expand All @@ -151,7 +154,6 @@ const (
)

func (a *auth) createCookie(name string, value string, httpOnly bool, expiresAt time.Time) *http.Cookie {

secure := true
sameSite := http.SameSiteNoneMode
domain := a.c.Registry.FQDN
Expand Down Expand Up @@ -192,10 +194,77 @@ func (a *auth) getUserWithGithubOauthToken(ctx context.Context, token string) (*
return nil, fmt.Errorf("GHO_UNAUTHORIZED")
}

user, err := a.pgStore.GetUser(ctx, oauthUser.Email, false)
user, err := a.pgStore.GetUser(ctx, oauthUser.Email, false, nil)
if err != nil {
return nil, fmt.Errorf("PG_GET_USER_ERR: %w", err)
}

return user, nil
}

func (a *auth) getGitHubErrorURI(status int, err string) string {
queryParams := url.Values{
"status": {fmt.Sprintf("%d", status)},
"error": {err},
}

return fmt.Sprintf("%s%s?%s", a.c.WebAppConfig.Endpoint, a.c.WebAppConfig.CallbackURL, queryParams.Encode())
}

func (a *auth) finishGitHubCallback(ctx echo.Context, username, userId string, oauthToken *oauth2.Token) error {
sessionId, err := uuid.NewRandom()
if err != nil {
return err
}

accessToken, refreshToken, err := a.SignOAuthToken(userId, oauthToken)
if err != nil {
return err
}

err = a.pgStore.AddSession(ctx.Request().Context(), sessionId.String(), refreshToken, username)
if err != nil {
return err
}

val := fmt.Sprintf("%s:%s", sessionId, userId)

sessionCookie := a.createCookie("session_id", val, false, time.Now().Add(time.Hour*750))
accessCookie := a.createCookie("access_token", accessToken, true, time.Now().Add(time.Hour*750))
refreshCookie := a.createCookie("refresh_token", refreshToken, true, time.Now().Add(time.Hour*750))

ctx.SetCookie(accessCookie)
ctx.SetCookie(refreshCookie)
ctx.SetCookie(sessionCookie)

return nil
}

func (a *auth) storeGitHubUserIfDoesntExist(ctx context.Context, pgErr error, user *types.User) error {
if errors.Unwrap(pgErr) == pgx.ErrNoRows {
id, err := uuid.NewRandom()
if err != nil {
return err
}
user.Id = id.String()
if err = user.Validate(false); err != nil {
return err
}

// In GitHub's response, Login is the GitHub Username
user.Username = user.Login
user.GithubConnected = true
if err = a.pgStore.AddOAuthUser(ctx, user); err != nil {
var pgErr *pgconn.PgError
// this would mean that the user email is already registered
// so we return an error in this case
if errors.As(err, &pgErr) && pgErr.Code == pgerrcode.UniqueViolation {
return fmt.Errorf("username/email already exists")
}
return err
}
return nil
}

return pgErr
}
Loading