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

✨ New probe for required MFA #4398

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add e2e tests of this on:

  1. repo owned by a user
  2. repo owned by an organization

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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Appears this is available on GitLab for group namespaces as well:
https://docs.gitlab.com/ee/api/groups.html

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)
Comment on lines 46 to +47
Copy link
Member

Choose a reason for hiding this comment

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

We may want to reach a decision on #4049

This is technically a breaking change.

GetOrgRepoClient(context.Context) (RepoClient, error)
ListCommits() ([]Commit, error)
ListIssues() ([]Issue, error)
Expand Down
16 changes: 12 additions & 4 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
pmc "github.com/ossf/scorecard/v5/cmd/internal/packagemanager"
docs "github.com/ossf/scorecard/v5/docs/checks"
sce "github.com/ossf/scorecard/v5/errors"
"github.com/ossf/scorecard/v5/finding"
sclog "github.com/ossf/scorecard/v5/log"
"github.com/ossf/scorecard/v5/options"
"github.com/ossf/scorecard/v5/pkg/scorecard"
Expand Down Expand Up @@ -164,9 +165,8 @@ func rootCmd(o *options.Options) error {

if o.Format == options.FormatDefault {
if len(enabledProbes) > 0 {
printProbeResults(enabledProbes)
} else {
printCheckResults(enabledChecks)
printProbeResults(enabledProbes, repoResult.Findings)
return nil
}
}

Expand All @@ -180,6 +180,7 @@ func rootCmd(o *options.Options) error {
return fmt.Errorf("failed to format results: %w", resultsErr)
}

printCheckResults(enabledChecks)
// intentionally placed at end to preserve outputting results, even if a check has a runtime error
for _, result := range repoResult.Checks {
if result.Error != nil {
Expand All @@ -201,10 +202,17 @@ func printCheckStart(enabledChecks checker.CheckNameToFnMap) {
}
}

func printProbeResults(enabledProbes []string) {
func printProbeResults(enabledProbes []string, findings []finding.Finding) {
for _, probeName := range enabledProbes {
fmt.Fprintf(os.Stderr, "Finished probe %s\n", probeName)
}
for _, result := range findings {
if result.Remediation != nil {
fmt.Fprintf(os.Stderr, "[%s] Remediation required: %s\n", result.Probe, result.Remediation.Text)
} else {
fmt.Fprintf(os.Stderr, "[%s] Passed: %s\n", result.Probe, result.Message)
}
}
}

func printCheckResults(enabledChecks checker.CheckNameToFnMap) {
Expand Down
15 changes: 15 additions & 0 deletions docs/probes.md
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,21 @@ If collaborators, members or owners have NOT participated in issues in the last
The probe returns 1 true outcome if the project has no workflows "write" permissions a the "job" level.


## orgRequiresMFA

**Lifecycle**: experimental

**Description**: A short description of this probe

**Motivation**: What is the motivation for this probe?

**Implementation**: How does this probe work under-the-hood?

**Outcomes**: 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


## packagedWithAutomatedWorkflow

**Lifecycle**: stable
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?
Comment on lines +18 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add these fields to the documentation?

outcome:
- If MFA is required by the organization that owns the repo, the probe returns OutcomeTrue
- IF MFA is not required by the organization that owns the repo, 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.
74 changes: 74 additions & 0 deletions probes/orgRequiresMFA/impl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// 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) {
if raw == nil {
err = fmt.Errorf("raw results is nil: %w", uerror.ErrNil)
return found, Probe, err
}

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

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 found, Probe, err
}

found = append(found, *result)

return found, Probe, err
}
103 changes: 103 additions & 0 deletions probes/orgRequiresMFA/impl_test.go

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

Loading