Skip to content

Commit

Permalink
✨ Added independent probe for required MFA
Browse files Browse the repository at this point in the history
Signed-off-by: Eddie Knight <[email protected]>
  • Loading branch information
eddie-knight committed Oct 29, 2024
1 parent 1703089 commit 1b9a198
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 1 deletion.
4 changes: 4 additions & 0 deletions clients/git/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,10 @@ func (c *Client) GetDefaultBranch() (*clients.BranchRef, error) {
return nil, clients.ErrUnsupportedFeature
}

func (c *Client) GetMFARequired() (required bool, err error) {
return required, clients.ErrUnsupportedFeature
}

func (c *Client) GetOrgRepoClient(ctx context.Context) (clients.RepoClient, error) {
return nil, clients.ErrUnsupportedFeature
}
Expand Down
11 changes: 11 additions & 0 deletions clients/githubrepo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,17 @@ func (client *Client) GetCreatedAt() (time.Time, error) {
return client.repo.CreatedAt.Time, nil
}

func (client *Client) GetMFARequired() (required bool, err error) {
org, _, err := client.repoClient.Organizations.Get(context.Background(), client.repourl.owner)
if err != nil {
return
}
if org.TwoFactorRequirementEnabled != nil {
return *org.TwoFactorRequirementEnabled, nil
}
return false, nil
}

func (client *Client) GetOrgRepoClient(ctx context.Context) (clients.RepoClient, error) {
dotGithubRepo, err := MakeGithubRepo(fmt.Sprintf("%s/.github", client.repourl.owner))
if err != nil {
Expand Down
5 changes: 5 additions & 0 deletions clients/gitlabrepo/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,11 @@ func (client *Client) GetCreatedAt() (time.Time, error) {
return client.project.getCreatedAt()
}

func (c *Client) GetMFARequired() (required bool, err error) {
err = fmt.Errorf("GetMFARequired: %w", clients.ErrUnsupportedFeature)
return
}

func (client *Client) GetOrgRepoClient(ctx context.Context) (clients.RepoClient, error) {
return nil, fmt.Errorf("GetOrgRepoClient (GitLab): %w", clients.ErrUnsupportedFeature)
}
Expand Down
5 changes: 5 additions & 0 deletions clients/localdir/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,11 @@ func (client *localDirClient) GetDefaultBranchName() (string, error) {
return "", fmt.Errorf("GetDefaultBranchName: %w", clients.ErrUnsupportedFeature)
}

func (c *localDirClient) GetMFARequired() (required bool, err error) {
err = fmt.Errorf("GetMFARequired: %w", clients.ErrUnsupportedFeature)
return
}

// ListCommits implements RepoClient.ListCommits.
func (client *localDirClient) ListCommits() ([]clients.Commit, error) {
return nil, fmt.Errorf("ListCommits: %w", clients.ErrUnsupportedFeature)
Expand Down
8 changes: 8 additions & 0 deletions clients/mockclients/repo_client.go

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

5 changes: 5 additions & 0 deletions clients/ossfuzz/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,11 @@ func (c *client) GetDefaultBranch() (*clients.BranchRef, error) {
return nil, fmt.Errorf("GetDefaultBranch: %w", clients.ErrUnsupportedFeature)
}

func (c *client) GetMFARequired() (required bool, err error) {
err = fmt.Errorf("GetMFARequired: %w", clients.ErrUnsupportedFeature)
return
}

// GetOrgRepoClient implements RepoClient.GetOrgRepoClient.
func (c *client) GetOrgRepoClient(ctx context.Context) (clients.RepoClient, error) {
return nil, fmt.Errorf("GetOrgRepoClient: %w", clients.ErrUnsupportedFeature)
Expand Down
1 change: 1 addition & 0 deletions clients/repo_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type RepoClient interface {
GetCreatedAt() (time.Time, error)
GetDefaultBranchName() (string, error)
GetDefaultBranch() (*BranchRef, error)
GetMFARequired() (bool, error)
GetOrgRepoClient(context.Context) (RepoClient, error)
ListCommits() ([]Commit, error)
ListIssues() ([]Issue, error)
Expand Down
5 changes: 4 additions & 1 deletion probes/entries.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/ossf/scorecard/v5/probes/hasUnverifiedBinaryArtifacts"
"github.com/ossf/scorecard/v5/probes/issueActivityByProjectMember"
"github.com/ossf/scorecard/v5/probes/jobLevelPermissions"
"github.com/ossf/scorecard/v5/probes/orgRequiresMFA"
"github.com/ossf/scorecard/v5/probes/packagedWithAutomatedWorkflow"
"github.com/ossf/scorecard/v5/probes/pinsDependencies"
"github.com/ossf/scorecard/v5/probes/releasesAreSigned"
Expand Down Expand Up @@ -173,7 +174,9 @@ var (
}

// Probes which don't use pre-computed raw data but rather collect it themselves.
Independent = []IndependentProbeImpl{}
Independent = []IndependentProbeImpl{
orgRequiresMFA.Run,
}
)

//nolint:gochecknoinits
Expand Down
30 changes: 30 additions & 0 deletions probes/orgRequiresMFA/def.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# Copyright 2024 OpenSSF Scorecard Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# 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.

id: orgRequiresMFA # required
lifecycle: experimental # required
short: A short description of this probe
motivation: >
What is the motivation for this probe?
implementation: >
How does this probe work under-the-hood?
outcome:
- If MFA is found to be required, the probe returns OutcomeTrue
- IF MFA is not found to be required, the probe returns OutcomeFalse
- If the runtime environment does not have authentication for the target project, the probe returns OutcomeNotAvailable
remediation:
onOutcome: False # required
effort: Low # required
text:
- In your project settings, require MFA for all collaborators.
76 changes: 76 additions & 0 deletions probes/orgRequiresMFA/impl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
// Copyright 2024 OpenSSF Scorecard Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// 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.

//nolint:stylecheck
package orgRequiresMFA

import (
"embed"
"fmt"

"github.com/ossf/scorecard/v5/checker"
"github.com/ossf/scorecard/v5/finding"
"github.com/ossf/scorecard/v5/internal/probes"
"github.com/ossf/scorecard/v5/probes/internal/utils/uerror"
)

//go:embed *.yml
var fs embed.FS

const (
Probe = "orgRequiresMFA"
)

func init() {
// Register independently of any checks
probes.MustRegisterIndependent(Probe, Run)
}

func Run(raw *checker.CheckRequest) (found []finding.Finding, probeName string, err error) {
probeName = Probe
if raw == nil {
err = fmt.Errorf("raw results is nil: %w", uerror.ErrNil)
return

Check failure on line 44 in probes/orgRequiresMFA/impl.go

View workflow job for this annotation

GitHub Actions / check-linter

naked return in func `Run` with 36 lines of code (nakedret)
}

mfaRequired, err := raw.RepoClient.GetMFARequired()
if err != nil {
err = fmt.Errorf("getting MFA required: %w", err)
return

Check failure on line 50 in probes/orgRequiresMFA/impl.go

View workflow job for this annotation

GitHub Actions / check-linter

naked return in func `Run` with 36 lines of code (nakedret)
}

var outcome finding.Outcome
if mfaRequired {
outcome = finding.OutcomeTrue
} else {
outcome = finding.OutcomeFalse
}

result, err := finding.NewWith(
fs,
Probe,
"Collaborators require MFA",
nil,
outcome,
)

if err != nil {
err = fmt.Errorf("creating finding: %w", err)
return

Check failure on line 70 in probes/orgRequiresMFA/impl.go

View workflow job for this annotation

GitHub Actions / check-linter

naked return in func `Run` with 36 lines of code (nakedret)
}

found = append(found, *result)

return

Check failure on line 75 in probes/orgRequiresMFA/impl.go

View workflow job for this annotation

GitHub Actions / check-linter

naked return in func `Run` with 36 lines of code (nakedret)
}
84 changes: 84 additions & 0 deletions probes/orgRequiresMFA/impl_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package orgRequiresMFA

Check failure on line 1 in probes/orgRequiresMFA/impl_test.go

View workflow job for this annotation

GitHub Actions / check-linter

ST1003: should not use MixedCaps in package name; orgRequiresMFA should be orgrequiresmfa (stylecheck)

import (
"errors"
"testing"

"github.com/ossf/scorecard/v5/checker"
mockrepo "github.com/ossf/scorecard/v5/clients/mockclients"
"github.com/ossf/scorecard/v5/finding"
"github.com/ossf/scorecard/v5/probes/internal/utils/uerror"
)

// ConfigurableMockRepoClient mocks RepoClient with configurable return values.
type ConfigurableMockRepoClient struct {

Check failure on line 14 in probes/orgRequiresMFA/impl_test.go

View workflow job for this annotation

GitHub Actions / check-linter

fieldalignment: struct with 40 pointer bytes could be 32 (govet)
mockrepo.MockRepoClient
MFARequired bool
ReturnError error
}

// GetMFARequired returns the pre-set value for MFA requirement.
func (m *ConfigurableMockRepoClient) GetMFARequired() (bool, error) {
return m.MFARequired, m.ReturnError
}

func (m *ConfigurableMockRepoClient) Close() error { return nil }

func TestProbeCodeApproved(t *testing.T) {

Check failure on line 27 in probes/orgRequiresMFA/impl_test.go

View workflow job for this annotation

GitHub Actions / check-linter

TestProbeCodeApproved's subtests should call t.Parallel (tparallel)
t.Parallel()
probeTests := []struct {

Check failure on line 29 in probes/orgRequiresMFA/impl_test.go

View workflow job for this annotation

GitHub Actions / check-linter

fieldalignment: struct with 56 pointer bytes could be 48 (govet)
name string
rawResults *checker.CheckRequest
expectedError error
expectedCount int
expectedOutcome finding.Outcome
}{
{
name: "mfa check succeeded",
rawResults: &checker.CheckRequest{
RepoClient: &ConfigurableMockRepoClient{MFARequired: true, ReturnError: nil},
},
expectedError: nil,
expectedCount: 1,
expectedOutcome: finding.OutcomeTrue,
},
{
name: "error retrieving MFA status",
rawResults: &checker.CheckRequest{
RepoClient: &ConfigurableMockRepoClient{ReturnError: uerror.ErrNil},
},
expectedError: uerror.ErrNil,
expectedCount: 0,
expectedOutcome: finding.OutcomeFalse,
},
{
name: "nil raw results",
rawResults: nil,
expectedError: uerror.ErrNil,
expectedCount: 0,
expectedOutcome: finding.OutcomeFalse,
},
}

for _, tt := range probeTests {

Check failure on line 63 in probes/orgRequiresMFA/impl_test.go

View workflow job for this annotation

GitHub Actions / check-linter

Range statement for test TestProbeCodeApproved missing the call to method parallel in test Run (paralleltest)
t.Run(tt.name, func(t *testing.T) {
found, probeName, err := Run(tt.rawResults)

if probeName != Probe {
t.Errorf("unexpected probe name: got %v, want %v", probeName, Probe)
}

if !errors.Is(err, tt.expectedError) {
t.Errorf("unexpected error: got %v, want %v", err, tt.expectedError)
}

if len(found) != tt.expectedCount {
t.Errorf("unexpected number of findings: got %d, want %d", len(found), tt.expectedCount)
}

if len(found) > 0 && found[0].Outcome != tt.expectedOutcome {
t.Errorf("unexpected outcome: got %v, want %v", found[0].Outcome, tt.expectedOutcome)
}
})
}
}

0 comments on commit 1b9a198

Please sign in to comment.