Skip to content

Commit

Permalink
Merge pull request #488 from containerish/fix/image-push-pull-access
Browse files Browse the repository at this point in the history
  • Loading branch information
@guacamole
guacamole authored Nov 25, 2023
2 parents 1e7c3f3 + 53c8731 commit 8f1ded0
Show file tree
Hide file tree
Showing 35 changed files with 946 additions and 528 deletions.
7 changes: 6 additions & 1 deletion auth/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import (

"github.com/containerish/OpenRegistry/config"
"github.com/containerish/OpenRegistry/services/email"
"github.com/containerish/OpenRegistry/store/v1/registry"
"github.com/containerish/OpenRegistry/store/v1/users"
"github.com/containerish/OpenRegistry/telemetry"
gh "github.com/google/go-github/v50/github"
gh "github.com/google/go-github/v56/github"
"github.com/labstack/echo/v4"
"golang.org/x/oauth2"
"golang.org/x/oauth2/github"
Expand All @@ -34,6 +35,7 @@ type Authentication interface {
ResetForgottenPassword(ctx echo.Context) error
ForgotPassword(ctx echo.Context) error
Invites(ctx echo.Context) error
RepositoryPermissionsMiddleware() echo.MiddlewareFunc
}

// New is the constructor function returns an Authentication implementation
Expand All @@ -44,6 +46,7 @@ func New(
sessionStore users.SessionStore,
emailStore users.EmailStore,
logger telemetry.Logger,
registryStore registry.RegistryStore,
) Authentication {
githubOAuth := &oauth2.Config{
ClientID: c.OAuth.Github.ClientID,
Expand All @@ -66,6 +69,7 @@ func New(
oauthStateStore: make(map[string]time.Time),
mu: &sync.RWMutex{},
emailClient: emailClient,
registryStore: registryStore,
}

go a.stateTokenCleanup()
Expand All @@ -86,6 +90,7 @@ type (
emailClient email.MailService
mu *sync.RWMutex
c *config.OpenRegistryConfig
registryStore registry.RegistryStore
}
)

Expand Down
144 changes: 71 additions & 73 deletions auth/basic_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"time"

"github.com/containerish/OpenRegistry/registry/v2"
"github.com/containerish/OpenRegistry/types"
"github.com/containerish/OpenRegistry/store/v1/types"
"github.com/labstack/echo/v4"
)

Expand All @@ -18,65 +18,52 @@ const (
AuthorizationHeaderKey = "Authorization"
)

//when we use JWT
/*AuthMiddleware
HTTP/1.1 401 Unauthorized
Content-Type: application/json; charset=utf-8
Docker-Distribution-Api-Version: registry/2.0
Www-Authenticate: Bearer realm="https://auth.docker.io/token",service="registry.docker.io",
scope="repository:samalba/my-app:pull,push"
Date: Thu, 10 Sep 2015 19:32:31 GMT
Content-Length: 235
Strict-Transport-Security: max-age=31536000
{"errors":[{"code":"UNAUTHORIZED","message":"","detail":}]}
*/
//var wwwAuthenticate = `Bearer realm="http://0.0.0.0:5000/auth/token",
//service="http://0.0.0.0:5000",scope="repository:%s`

// BasicAuth returns a middleware which in turn can be used to perform http basic auth
func (a *auth) BasicAuth() echo.MiddlewareFunc {
return a.BasicAuthWithConfig()
}

func (a *auth) buildBasicAuthenticationHeader(repoNamespace string) string {
return fmt.Sprintf(
"Bearer realm=%s,service=%s,scope=repository:%s:pull,push",
"Bearer realm=%s,service=%s,scope=%s",
strconv.Quote(fmt.Sprintf("%s/token", a.c.Endpoint())),
strconv.Quote(a.c.Endpoint()),
strconv.Quote(fmt.Sprintf("repository:%s:pull,push", repoNamespace)),
)
}

func (a *auth) checkJWT(authHeader string, cookies []*http.Cookie) bool {
parts := strings.Split(authHeader, " ")
if len(parts) == 2 {
return strings.EqualFold(parts[0], "Bearer")
}

// fallback to check for auth header in cookies
for _, cookie := range cookies {
if cookie.Name == AccessCookieKey {
// early return if access_token is found in cookies
// early return if access_token is found in cookies, this will be checked by the JWT middlware and not the
// basic auth middleware
return true
}
}

parts := strings.Split(authHeader, " ")
if len(parts) != 2 {
return false
}

return strings.EqualFold(parts[0], "Bearer")
return false
}

const (
defaultRealm = "Restricted"
authScheme = "Bearer"
defaultRealm = "Restricted"
authScheme = "Bearer"
authSchemeBasic = "Basic"
)

// BasicAuthConfig is a local copy of echo's middleware.BasicAuthWithConfig
func (a *auth) BasicAuthWithConfig() echo.MiddlewareFunc {

return func(next echo.HandlerFunc) echo.HandlerFunc {
return func(handler echo.HandlerFunc) echo.HandlerFunc {
return func(ctx echo.Context) error {

if a.SkipBasicAuth(ctx) {
return next(ctx)
a.logger.Debug().Bool("skip_basic_auth", true).Send()
// Note: there might be other middlewares attached to this handler
return handler(ctx)
}

auth := ctx.Request().Header.Get(echo.HeaderAuthorization)
Expand All @@ -85,6 +72,7 @@ func (a *auth) BasicAuthWithConfig() echo.MiddlewareFunc {
if len(auth) > l+1 && strings.EqualFold(auth[:l], authScheme) {
b, err := base64.StdEncoding.DecodeString(auth[l+1:])
if err != nil {
a.logger.Debug().Err(err).Send()
return err
}
cred := string(b)
Expand All @@ -93,61 +81,46 @@ func (a *auth) BasicAuthWithConfig() echo.MiddlewareFunc {
// Verify credentials
valid, err := a.BasicAuthValidator(cred[:i], cred[i+1:], ctx)
if err != nil {
a.logger.Debug().Err(err).Send()
return err
} else if valid {
return next(ctx)
return handler(ctx)
}
break
}
}
}

headerValue := fmt.Sprintf("Bearer realm=%s", strconv.Quote(fmt.Sprintf("%s/token", a.c.Endpoint())))
namespace, ok := ctx.Get(string(registry.RegistryNamespace)).(string)
if ok {
headerValue = a.buildBasicAuthenticationHeader(namespace)
username := ctx.Param("username")
imageName := ctx.Param("imagename")
if username != "" && imageName != "" {
headerValue = a.buildBasicAuthenticationHeader(username + "/" + imageName)
}

// Need to return `401` for browsers to pop-up login box.
ctx.Response().Header().Set(echo.HeaderWWWAuthenticate, headerValue)
return echo.ErrUnauthorized
echoErr := ctx.NoContent(http.StatusUnauthorized)
a.logger.Log(ctx, nil).Send()
return echoErr
}
}
}

func (a *auth) BasicAuthValidator(username string, password string, ctx echo.Context) (bool, error) {
ctx.Set(types.HandlerStartTime, time.Now())

if ctx.Request().URL.Path == "/v2/" {
_, err := a.validateUser(username, password)
if err != nil {
echoErr := ctx.NoContent(http.StatusUnauthorized)
a.logger.Log(ctx, err).Send()
return false, echoErr
}

return true, nil
}

usernameFromNameSpace := ctx.Param("username")
if usernameFromNameSpace != username {
var errMsg registry.RegistryErrors
errMsg.Errors = append(errMsg.Errors, registry.RegistryError{
Code: registry.RegistryErrorCodeDenied,
Message: "user is not authorised to perform this action",
Detail: nil,
})
echoErr := ctx.JSON(http.StatusForbidden, errMsg)
a.logger.Log(ctx, fmt.Errorf("%s", errMsg)).Send()
return false, echoErr
}
_, err := a.validateUser(username, password)
if err != nil {
usernameFromReq := ctx.Param("username")
if err != nil || usernameFromReq != username {
var errMsg registry.RegistryErrors
errMsg.Errors = append(errMsg.Errors, registry.RegistryError{
Code: registry.RegistryErrorCodeDenied,
Message: err.Error(),
Detail: nil,
Code: registry.RegistryErrorCodeUnauthorized,
Message: "user is not authorized to perform this action",
Detail: echo.Map{
"reason": "you are not allowed to push to this account, please check if you are logged in with the right user.",
"error": err.Error(),
},
})
echoErr := ctx.JSON(http.StatusUnauthorized, errMsg)
a.logger.Log(ctx, fmt.Errorf("%s", errMsg)).Send()
Expand All @@ -158,23 +131,48 @@ func (a *auth) BasicAuthValidator(username string, password string, ctx echo.Con
}

func (a *auth) SkipBasicAuth(ctx echo.Context) bool {
authHeader := ctx.Request().Header.Get(AuthorizationHeaderKey)
authHeader := ctx.Request().Header.Get(echo.HeaderAuthorization)

// if found, populate requested repository in request context, so that any of the chained middlwares can
// read the value from ctx instead of database
repo := a.tryPopulateRepository(ctx)

// if Authorization header contains JWT, we skip basic auth and perform a JWT validation
if ok := a.checkJWT(authHeader, ctx.Request().Cookies()); ok {
ctx.Set(JWT_AUTH_KEY, true)
a.logger.Debug().
Bool("skip_basic_auth", true).
Str("method", ctx.Request().Method).
Str("path", ctx.Request().URL.RequestURI()).
Send()
return true
}

if ctx.Request().URL.Path != "/v2/" {
if ctx.Request().Method == http.MethodHead || ctx.Request().Method == http.MethodGet {
return true
}
}

if ctx.Request().URL.Path == "/v2/" {
return false
readOp := ctx.Request().Method == http.MethodHead || ctx.Request().Method == http.MethodGet
// if it's a read operation on a public repository, we skip auth requirement
if readOp && repo != nil && repo.Visibility == types.RepositoryVisibilityPublic {
a.logger.Debug().
Bool("skip_basic_auth", true).
Str("method", ctx.Request().Method).
Str("path", ctx.Request().URL.RequestURI()).
Send()
return true
}

return false
}

func (a *auth) tryPopulateRepository(ctx echo.Context) *types.ContainerImageRepository {
if strings.HasPrefix(ctx.Request().URL.Path, "/v2/") {
username := ctx.Param("username")
imageName := ctx.Param("imagename")
if username != "" && imageName != "" {
ns := username + "/" + imageName
repo, err := a.registryStore.GetRepositoryByNamespace(ctx.Request().Context(), ns)
if err == nil {
ctx.Set(string(types.UserRepositoryContextKey), repo)
return repo
}
}
}
return nil
}
31 changes: 14 additions & 17 deletions auth/github.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,17 @@ package auth

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

"github.com/containerish/OpenRegistry/config"
v2_types "github.com/containerish/OpenRegistry/store/v1/types"
"github.com/containerish/OpenRegistry/types"
"github.com/google/go-github/v53/github"
"github.com/google/go-github/v56/github"
"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 @@ -38,7 +35,7 @@ func (a *auth) LoginWithGithub(ctx echo.Context) error {
a.mu.Unlock()
url := a.github.AuthCodeURL(state.String(), oauth2.AccessTypeOffline)
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, url)
a.logger.Log(ctx, echoErr).Send()
a.logger.Log(ctx, nil).Send()
return echoErr
}

Expand Down Expand Up @@ -91,22 +88,22 @@ func (a *auth) GithubLoginCallbackHandler(ctx echo.Context) error {
user, err := a.pgStore.GetGitHubUser(ctx.Request().Context(), ghUser.GetEmail(), nil)
if err != nil {
user = user.NewUserFromGitHubUser(ghUser)
err = a.storeGitHubUserIfDoesntExist(ctx.Request().Context(), err, user)
if err != nil {
uri := a.getGitHubErrorURI(ctx, http.StatusConflict, err.Error())
storeErr := a.storeGitHubUserIfDoesntExist(ctx.Request().Context(), err, user)
if storeErr != nil {
uri := a.getGitHubErrorURI(ctx, http.StatusConflict, storeErr.Error())
echoErr := ctx.Redirect(http.StatusSeeOther, uri)
a.logger.Log(ctx, err).Send()
a.logger.Log(ctx, storeErr).Send()
return echoErr
}
if err = a.finishGitHubCallback(ctx, user, token); err != nil {
uri := a.getGitHubErrorURI(ctx, http.StatusConflict, err.Error())
if callbackErr := a.finishGitHubCallback(ctx, user, token); callbackErr != nil {
uri := a.getGitHubErrorURI(ctx, http.StatusConflict, callbackErr.Error())
echoErr := ctx.Redirect(http.StatusTemporaryRedirect, uri)
a.logger.Log(ctx, err).Send()
a.logger.Log(ctx, callbackErr).Send()
return echoErr
}

err = ctx.Redirect(http.StatusTemporaryRedirect, a.c.WebAppConfig.RedirectURL)
a.logger.Log(ctx, nil).Send()
a.logger.Log(ctx, err).Send()
return err
}

Expand Down Expand Up @@ -283,7 +280,7 @@ func (a *auth) finishGitHubCallback(ctx echo.Context, user *v2_types.User, oauth
}

func (a *auth) storeGitHubUserIfDoesntExist(ctx context.Context, pgErr error, user *v2_types.User) error {
if errors.Unwrap(pgErr) == pgx.ErrNoRows {
if strings.HasSuffix(pgErr.Error(), "no rows in result set") {
id, err := uuid.NewRandom()
if err != nil {
return err
Expand All @@ -295,10 +292,10 @@ func (a *auth) storeGitHubUserIfDoesntExist(ctx context.Context, pgErr error, us

// In GitHub's response, Login is the GitHub Username
if err = a.pgStore.AddUser(ctx, user, nil); 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 {

if strings.Contains(err.Error(), "duplicate key value violation") {
return fmt.Errorf("username/email already exists")
}
return err
Expand Down
4 changes: 2 additions & 2 deletions auth/jwt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ type ServiceClaims struct {
Access AccessList
}

func (a *auth) newPublicPullToken() (string, error) {
func (a *auth) newPublicPullToken(userId string) (string, error) {
acl := AccessList{
{
Type: "repository",
Expand All @@ -49,7 +49,7 @@ func (a *auth) newPublicPullToken() (string, error) {
opts := &CreateClaimOptions{
Audience: a.c.Registry.FQDN,
Issuer: OpenRegistryIssuer,
Id: "public_pull_user",
Id: userId,
TokeType: "service_token",
Acl: acl,
}
Expand Down
Loading

0 comments on commit 8f1ded0

Please sign in to comment.