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

Bed-4851 OIDC API Provider Registration #894

Merged
merged 23 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
97eb109
BED-4851 Updated CreateOIDCProvider endpoint and relevant database in…
mvlipka Sep 30, 2024
e7111dd
BED-4851 updates to conform to new design
mvlipka Oct 1, 2024
2a22dc6
BED-4851 cleanup import
mvlipka Oct 1, 2024
5028c18
BED-4851 added integration test for TestBloodhoundDB_CreateSSOProvide…
mvlipka Oct 2, 2024
686a359
BED-4851 removed name from oidc_providers
mvlipka Oct 2, 2024
6a67c61
BED-4851 updated tests with removed name field
mvlipka Oct 2, 2024
4c818e7
BED-4851 idempotent saml_providers constraint
mvlipka Oct 2, 2024
0723780
BED-4851 moved responsibility of slug creation to the db layer. Prope…
mvlipka Oct 3, 2024
022d2a6
Merge main
mvlipka Oct 3, 2024
6877860
generate
mvlipka Oct 3, 2024
704c330
removed test I was playing with and accidentally committed, oops. Add…
mvlipka Oct 3, 2024
e55f5e0
fix returning error from CreateSSOPRovider
mvlipka Oct 3, 2024
1cf416d
added test for invalid SessionAuthProvider
mvlipka Oct 3, 2024
0be0354
fixed test, better naming on sso provider types
mvlipka Oct 3, 2024
5b4edcc
removed no longer used SetupTransaction
mvlipka Oct 4, 2024
c98ff3e
fixed bug in listing audit logs. Added sso_provider_id to users table…
mvlipka Oct 7, 2024
2555790
fixed bug in listing audit logs
mvlipka Oct 7, 2024
76e0252
revert re-ordering auditlog time parameters
mvlipka Oct 7, 2024
f2ebadd
Merge branch 'main' into BED-4851-oidc-api-provider-registration-2
mvlipka Oct 7, 2024
1795e78
make sso_provider_id nullable on SAMLProvider and add sso_provider_id…
mvlipka Oct 7, 2024
c828f5f
backfill users with their appropriate sso_provider_id
mvlipka Oct 8, 2024
8157ef4
removed new constant for SSO Provider and revert back to AuthProvider
mvlipka Oct 8, 2024
87898f1
added client_id to audit logs and no longer delete users when their s…
mvlipka Oct 9, 2024
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
2 changes: 1 addition & 1 deletion cmd/api/src/api/registration/v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func registerV2Auth(cfg config.Configuration, db database.Database, permissions
routerInst.DELETE(fmt.Sprintf("/api/v2/saml/providers/{%s}", api.URIPathVariableSAMLProviderID), managementResource.DeleteSAMLProvider).RequirePermissions(permissions.AuthManageProviders),

// SSO
routerInst.POST("/api/v2/sso/providers/oidc", managementResource.CreateOIDCProvider).CheckFeatureFlag(db, appcfg.FeatureOIDCSupport).RequirePermissions(permissions.AuthManageProviders),
routerInst.POST("/api/v2/sso-providers/oidc", managementResource.CreateOIDCProvider).CheckFeatureFlag(db, appcfg.FeatureOIDCSupport).RequirePermissions(permissions.AuthManageProviders),

// Permissions
routerInst.GET("/api/v2/permissions", managementResource.ListPermissions).RequirePermissions(permissions.AuthManageSelf),
Expand Down
8 changes: 3 additions & 5 deletions cmd/api/src/api/v2/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,15 @@ func (s ManagementResource) CreateOIDCProvider(response http.ResponseWriter, req
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, err.Error(), request), response)
} else if validated := validation.Validate(createRequest); validated != nil {
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, validated.Error(), request), response)
} else if strings.Contains(createRequest.Name, " ") {
api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, "invalid name formatting, ensure there are no spaces in the provided name", request), response)
} else {
var (
formattedName = strings.ToLower(createRequest.Name)
formattedName = strings.ToLower(strings.ReplaceAll(createRequest.Name, " ", "-"))
mvlipka marked this conversation as resolved.
Show resolved Hide resolved
)

if provider, err := s.db.CreateOIDCProvider(request.Context(), formattedName, createRequest.Issuer, createRequest.ClientID); err != nil {
if oidcProvider, err := s.db.CreateOIDCProvider(request.Context(), createRequest.Name, formattedName, createRequest.Issuer, createRequest.ClientID); err != nil {
api.HandleDatabaseError(request, response, err)
} else {
api.WriteBasicResponse(request.Context(), provider, http.StatusCreated, response)
api.WriteBasicResponse(request.Context(), oidcProvider, http.StatusCreated, response)
}
}
}
34 changes: 14 additions & 20 deletions cmd/api/src/api/v2/auth/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,12 @@ package auth_test
import (
"fmt"
"net/http"

"github.com/specterops/bloodhound/src/model"

"github.com/specterops/bloodhound/src/api/v2/auth"
"testing"

"github.com/specterops/bloodhound/src/api/v2/apitest"
"github.com/specterops/bloodhound/src/api/v2/auth"
"github.com/specterops/bloodhound/src/model"
"github.com/specterops/bloodhound/src/utils/test"

"testing"

"go.uber.org/mock/gomock"
)

Expand All @@ -43,19 +39,17 @@ func TestManagementResource_CreateOIDCProvider(t *testing.T) {
defer mockCtrl.Finish()

t.Run("successfully create a new OIDCProvider", func(t *testing.T) {
mockDB.EXPECT().CreateOIDCProvider(gomock.Any(), "test", "https://localhost/auth", "bloodhound").Return(model.OIDCProvider{
Name: "",
ClientID: "",
Issuer: "",
mockDB.EXPECT().CreateOIDCProvider(gomock.Any(), "Bloodhound gang", "bloodhound-gang", "https://localhost/auth", "bloodhound").Return(model.OIDCProvider{
ClientID: "bloodhound",
Issuer: "https://localhost/auth",
}, nil)

test.Request(t).
WithMethod(http.MethodPost).
WithURL(url).
WithBody(auth.CreateOIDCProviderRequest{
Name: "test",
Issuer: "https://localhost/auth",

Name: "Bloodhound gang",
Issuer: "https://localhost/auth",
ClientID: "bloodhound",
}).
OnHandlerFunc(resources.CreateOIDCProvider).
Expand All @@ -78,8 +72,9 @@ func TestManagementResource_CreateOIDCProvider(t *testing.T) {
WithMethod(http.MethodPost).
WithURL(url).
WithBody(auth.CreateOIDCProviderRequest{
Name: "test",
Issuer: "",
Name: "test",
Issuer: "1234:not:a:url",
ClientID: "bloodhound",
}).
OnHandlerFunc(resources.CreateOIDCProvider).
Require().
Expand All @@ -100,15 +95,14 @@ func TestManagementResource_CreateOIDCProvider(t *testing.T) {
})

t.Run("error creating oidc provider db entry", func(t *testing.T) {
mockDB.EXPECT().CreateOIDCProvider(gomock.Any(), "test", "https://localhost/auth", "bloodhound").Return(model.OIDCProvider{}, fmt.Errorf("error"))
mockDB.EXPECT().CreateOIDCProvider(gomock.Any(), "test", "test", "https://localhost/auth", "bloodhound").Return(model.OIDCProvider{}, fmt.Errorf("error"))

test.Request(t).
WithMethod(http.MethodPost).
WithURL(url).
WithBody(auth.CreateOIDCProviderRequest{
Name: "test",
Issuer: "https://localhost/auth",

Name: "test",
Issuer: "https://localhost/auth",
ClientID: "bloodhound",
}).
OnHandlerFunc(resources.CreateOIDCProvider).
Expand Down
1 change: 1 addition & 0 deletions cmd/api/src/database/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ type Database interface {
DeleteSAMLProvider(ctx context.Context, samlProvider model.SAMLProvider) error

// SSO
SSOProviderData
OIDCProviderData

// Sessions
Expand Down
52 changes: 44 additions & 8 deletions cmd/api/src/database/migration/migrations/v6.1.0.sql
Original file line number Diff line number Diff line change
@@ -1,13 +1,49 @@
-- Copyright 2024 Specter Ops, Inc.
--
-- Licensed under the Apache License, Version 2.0
-- you may not use this file except in compliance with the License.
-- You may obtain a copy of the License at
--
-- http://www.apache.org/licenses/LICENSE-2.0
--
-- Unless required by applicable law or agreed to in writing, software
-- distributed under the License is distributed on an "AS IS" BASIS,
-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-- See the License for the specific language governing permissions and
-- limitations under the License.
--
-- SPDX-License-Identifier: Apache-2.0

-- SSO Provider
CREATE TABLE IF NOT EXISTS sso_providers
(
id SERIAL PRIMARY KEY,
name TEXT NOT NULL,
slug TEXT NOT NULL,
type int NOT NULL,
mvlipka marked this conversation as resolved.
Show resolved Hide resolved

updated_at TIMESTAMP WITH TIME ZONE DEFAULT now(),
created_at TIMESTAMP WITH TIME ZONE DEFAULT now(),

UNIQUE (name),
UNIQUE (slug)
);

-- OIDC Provider
CREATE TABLE IF NOT EXISTS oidc_providers
(
id BIGSERIAL PRIMARY KEY,
name TEXT NOT NULL,
client_id TEXT NOT NULL,
issuer TEXT NOT NULL,
id SERIAL PRIMARY KEY,
client_id TEXT NOT NULL,
issuer TEXT NOT NULL,
sso_provider_id INTEGER REFERENCES sso_providers (id) ON DELETE CASCADE NULL,
ddlees marked this conversation as resolved.
Show resolved Hide resolved

updated_at TIMESTAMP WITH TIME ZONE DEFAULT now(),
created_at TIMESTAMP WITH TIME ZONE DEFAULT now()
);

updated_at TIMESTAMP WITH TIME ZONE DEFAULT now(),
created_at TIMESTAMP WITH TIME ZONE DEFAULT now(),
-- Create the reference from saml_providers to sso_providers
ALTER TABLE ONLY saml_providers
ADD COLUMN IF NOT EXISTS sso_provider_id INTEGER NULL;
ALTER TABLE ONLY saml_providers
ADD CONSTRAINT fk_saml_provider_sso_provider FOREIGN KEY (sso_provider_id) REFERENCES sso_providers (id) ON DELETE CASCADE;
mvlipka marked this conversation as resolved.
Show resolved Hide resolved

UNIQUE (name)
)
39 changes: 35 additions & 4 deletions cmd/api/src/database/mocks/db.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 21 additions & 6 deletions cmd/api/src/database/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,35 @@ import (
"context"

"github.com/specterops/bloodhound/src/model"
"gorm.io/gorm"
)

const (
oidcProvidersTableName = "oidc_providers"
)

// OIDCProviderData defines the interface required to interact with the oidc_providers table
type OIDCProviderData interface {
CreateOIDCProvider(ctx context.Context, name, issuer, clientID string) (model.OIDCProvider, error)
CreateOIDCProvider(ctx context.Context, name, slug, issuer, clientID string) (model.OIDCProvider, error)
}

// CreateOIDCProvider creates a new entry for an OIDC provider
func (s *BloodhoundDB) CreateOIDCProvider(ctx context.Context, name, issuer, clientID string) (model.OIDCProvider, error) {
provider := model.OIDCProvider{
Name: name,
// CreateOIDCProvider creates a new entry for an OIDC provider as well as the associated SSO provider
func (s *BloodhoundDB) CreateOIDCProvider(ctx context.Context, name, slug, issuer, clientID string) (model.OIDCProvider, error) {
oidcProvider := model.OIDCProvider{
ddlees marked this conversation as resolved.
Show resolved Hide resolved
ClientID: clientID,
Issuer: issuer,
}

return provider, CheckError(s.db.WithContext(ctx).Table("oidc_providers").Create(&provider))
// Create both the sso_providers and oidc_providers rows in a single transaction
// If one of these requests errors, both changes will be rolled back
err := s.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not the BIGGEST fan of intermingling the CreateSSOProvider and CreateOIDCProvider methods since the end-user may simply want to just create an oidc_provider, for whatever reason. With our usecase, though, these tables are required to be in sync with each other so I felt comfortable doing this

if ssoProvider, err := s.CreateSSOProviderWithTransaction(ctx, tx, name, slug, model.SessionAuthProviderOIDC); err != nil {
return err
} else {
oidcProvider.SSOProviderID = int(ssoProvider.ID)
return CheckError(tx.WithContext(ctx).Table(oidcProvidersTableName).Create(&oidcProvider))
}
})

return oidcProvider, err
}
3 changes: 1 addition & 2 deletions cmd/api/src/database/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,9 @@ func TestBloodhoundDB_CreateOIDCProvider(t *testing.T) {
defer dbInst.Close(testCtx)

t.Run("successfully create an OIDC provider", func(t *testing.T) {
provider, err := dbInst.CreateOIDCProvider(testCtx, "test", "https://test.localhost.com/auth", "bloodhound")
provider, err := dbInst.CreateOIDCProvider(testCtx, "test", "test", "https://test.localhost.com/auth", "bloodhound")
require.NoError(t, err)

assert.Equal(t, "test", provider.Name)
assert.Equal(t, "https://test.localhost.com/auth", provider.Issuer)
assert.Equal(t, "bloodhound", provider.ClientID)
})
Expand Down
56 changes: 56 additions & 0 deletions cmd/api/src/database/sso.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// Copyright 2024 Specter Ops, Inc.
//
// Licensed under the Apache License, Version 2.0
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
// SPDX-License-Identifier: Apache-2.0

package database

import (
"context"

"github.com/specterops/bloodhound/src/model"
"gorm.io/gorm"
)

const (
ssoProviderTableName = "sso_providers"
)

// SSOProviderData defines the methods required to interact with the sso_providers table
type SSOProviderData interface {
CreateSSOProvider(ctx context.Context, name, slug string, authType model.SessionAuthProvider) (model.SSOProvider, error)
CreateSSOProviderWithTransaction(ctx context.Context, tx *gorm.DB, name, slug string, authType model.SessionAuthProvider) (model.SSOProvider, error)
}

// CreateSSOProvider creates an entry in the sso_providers table
func (s *BloodhoundDB) CreateSSOProvider(ctx context.Context, name, slug string, authType model.SessionAuthProvider) (model.SSOProvider, error) {
provider := model.SSOProvider{
Name: name,
Slug: slug,
mvlipka marked this conversation as resolved.
Show resolved Hide resolved
Type: authType,
}

return provider, CheckError(s.db.WithContext(ctx).Table(ssoProviderTableName).Create(&provider))
}

// CreateSSOProviderWithTransaction uses a db transaction to create an entry in the sso_providers table
func (s *BloodhoundDB) CreateSSOProviderWithTransaction(ctx context.Context, tx *gorm.DB, name, slug string, authType model.SessionAuthProvider) (model.SSOProvider, error) {
mvlipka marked this conversation as resolved.
Show resolved Hide resolved
provider := model.SSOProvider{
Name: name,
Slug: slug,
Type: authType,
}

return provider, CheckError(tx.WithContext(ctx).Table(ssoProviderTableName).Create(&provider))
}
Loading
Loading