Skip to content

Commit

Permalink
Generalizing SAMLOfflineSigninLimiters
Browse files Browse the repository at this point in the history
This CL changes the names of the SAMLOfflineSigninLimiter classes
to be more generic after having extended the classes to include
GAIA logic.

Bug: b/179637649

Change-Id: I2d8cafd12ef17c3168da3be4e19151a25770c4e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2692529
Commit-Queue: Aya Nader Elgendy‎ <[email protected]>
Reviewed-by: Denis Kuznetsov [CET] <[email protected]>
Cr-Commit-Position: refs/heads/master@{#855254}
  • Loading branch information
ayaNader authored and Chromium LUCI CQ committed Feb 18, 2021
1 parent fbc7546 commit a516967
Show file tree
Hide file tree
Showing 11 changed files with 193 additions and 197 deletions.
10 changes: 5 additions & 5 deletions chrome/browser/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1846,10 +1846,6 @@ source_set("chromeos") {
"login/saml/public_saml_url_fetcher.h",
"login/saml/saml_metric_utils.cc",
"login/saml/saml_metric_utils.h",
"login/saml/saml_offline_signin_limiter.cc",
"login/saml/saml_offline_signin_limiter.h",
"login/saml/saml_offline_signin_limiter_factory.cc",
"login/saml/saml_offline_signin_limiter_factory.h",
"login/saml/saml_profile_prefs.cc",
"login/saml/saml_profile_prefs.h",
"login/screen_manager.cc",
Expand Down Expand Up @@ -1983,6 +1979,10 @@ source_set("chromeos") {
"login/signin/oauth2_token_fetcher.h",
"login/signin/oauth2_token_initializer.cc",
"login/signin/oauth2_token_initializer.h",
"login/signin/offline_signin_limiter.cc",
"login/signin/offline_signin_limiter.h",
"login/signin/offline_signin_limiter_factory.cc",
"login/signin/offline_signin_limiter_factory.h",
"login/signin/signin_error_notifier_ash.cc",
"login/signin/signin_error_notifier_ash.h",
"login/signin/signin_error_notifier_factory_ash.cc",
Expand Down Expand Up @@ -3803,13 +3803,13 @@ source_set("unit_tests") {
"login/saml/password_expiry_notification_unittest.cc",
"login/saml/password_sync_token_login_checker_unittest.cc",
"login/saml/password_sync_token_verifier_unittest.cc",
"login/saml/saml_offline_signin_limiter_unittest.cc",
"login/screens/encryption_migration_screen_unittest.cc",
"login/screens/network_screen_unittest.cc",
"login/screens/recommend_apps/recommend_apps_fetcher_impl_unittest.cc",
"login/screens/update_required_screen_unittest.cc",
"login/screens/update_screen_unittest.cc",
"login/session/user_session_manager_test.cc",
"login/signin/offline_signin_limiter_unittest.cc",
"login/signin/signin_error_notifier_ash_unittest.cc",
"login/signin_partition_manager_unittest.cc",
"login/ui/login_screen_extension_ui/dialog_delegate_unittest.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ TEST_F(ExistingUserControllerForcedOnlineAuthTest, SamlOnlineAuthSingleUser) {
EXPECT_TRUE(is_force_online_flag_set());
}

// Verfies that `SAMLOfflineSigninLimiter` does affect SAML and non SAML user.
// Verfies that `OfflineSigninLimiter` does affect SAML and non SAML user.
TEST_F(ExistingUserControllerForcedOnlineAuthTest,
OfflineLimiteOutOfSessionSAMLAndNonSAML) {
const base::Time now = base::DefaultClock::GetInstance()->Now();
Expand Down

This file was deleted.

This file was deleted.

7 changes: 4 additions & 3 deletions chrome/browser/chromeos/login/saml/saml_profile_prefs.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@
namespace {

// The value -1 means that online authentication will not be enforced by
// `SAMLOfflineSigninLimiter` so the user will be allowed to use offline
// `OfflineSigninLimiter` so the user will be allowed to use offline
// authentication until a different reason than this policy enforces an online
// login.
const int kDefaultGaiaOfflineSigninTimeLimitDays = -1;
const int kDefaultSAMLOfflineSigninTimeLimit = 14 * 24 * 60 * 60; // 14 days.
constexpr int kDefaultGaiaOfflineSigninTimeLimitDays = -1;
constexpr int kDefaultSAMLOfflineSigninTimeLimit =
base::TimeDelta::FromDays(14).InSeconds();

// In-session password-change feature (includes password expiry notifications).
const bool kDefaultSamlInSessionPasswordChangeEnabled = false;
Expand Down
12 changes: 6 additions & 6 deletions chrome/browser/chromeos/login/session/user_session_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,12 @@
#include "chrome/browser/chromeos/login/quick_unlock/pin_backend.h"
#include "chrome/browser/chromeos/login/saml/password_sync_token_verifier.h"
#include "chrome/browser/chromeos/login/saml/password_sync_token_verifier_factory.h"
#include "chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.h"
#include "chrome/browser/chromeos/login/saml/saml_offline_signin_limiter_factory.h"
#include "chrome/browser/chromeos/login/screens/arc_terms_of_service_screen.h"
#include "chrome/browser/chromeos/login/screens/sync_consent_screen.h"
#include "chrome/browser/chromeos/login/session/user_session_initializer.h"
#include "chrome/browser/chromeos/login/signin/oauth2_login_manager_factory.h"
#include "chrome/browser/chromeos/login/signin/offline_signin_limiter.h"
#include "chrome/browser/chromeos/login/signin/offline_signin_limiter_factory.h"
#include "chrome/browser/chromeos/login/signin/token_handle_fetcher.h"
#include "chrome/browser/chromeos/login/startup_utils.h"
#include "chrome/browser/chromeos/login/ui/input_events_blocker.h"
Expand Down Expand Up @@ -1621,10 +1621,10 @@ void UserSessionManager::FinalizePrepareProfile(Profile* profile) {
}
}

SAMLOfflineSigninLimiter* saml_offline_signin_limiter =
SAMLOfflineSigninLimiterFactory::GetForProfile(profile);
if (saml_offline_signin_limiter)
saml_offline_signin_limiter->SignedIn(user_context_.GetAuthFlow());
OfflineSigninLimiter* offline_signin_limiter =
OfflineSigninLimiterFactory::GetForProfile(profile);
if (offline_signin_limiter)
offline_signin_limiter->SignedIn(user_context_.GetAuthFlow());
}

profile->OnLogin();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/chromeos/login/saml/saml_offline_signin_limiter.h"
#include "chrome/browser/chromeos/login/signin/offline_signin_limiter.h"

#include <string>
#include <utility>
Expand Down Expand Up @@ -37,9 +37,9 @@ constexpr int kSAMLOfflineSigninTimeLimitNotSet = -1;
// policy's definition.
constexpr int kGaiaOfflineSigninTimeLimitDaysNotSet = -1;

}
} // namespace

void SAMLOfflineSigninLimiter::SignedIn(UserContext::AuthFlow auth_flow) {
void OfflineSigninLimiter::SignedIn(UserContext::AuthFlow auth_flow) {
PrefService* prefs = profile_->GetPrefs();
const user_manager::User* user =
ProfileHelper::Get()->GetUserByProfile(profile_);
Expand Down Expand Up @@ -92,11 +92,11 @@ void SAMLOfflineSigninLimiter::SignedIn(UserContext::AuthFlow auth_flow) {
pref_change_registrar_.Init(prefs);
pref_change_registrar_.Add(
prefs::kSAMLOfflineSigninTimeLimit,
base::BindRepeating(&SAMLOfflineSigninLimiter::UpdateLimit,
base::BindRepeating(&OfflineSigninLimiter::UpdateLimit,
base::Unretained(this)));
pref_change_registrar_.Add(
prefs::kGaiaOfflineSigninTimeLimitDays,
base::BindRepeating(&SAMLOfflineSigninLimiter::UpdateLimit,
base::BindRepeating(&OfflineSigninLimiter::UpdateLimit,
base::Unretained(this)));
// Start listening to power state.
base::PowerMonitor::AddObserver(this);
Expand All @@ -112,41 +112,40 @@ void SAMLOfflineSigninLimiter::SignedIn(UserContext::AuthFlow auth_flow) {
UpdateLimit();
}

void SAMLOfflineSigninLimiter::SetTimerForTesting(
void OfflineSigninLimiter::SetTimerForTesting(
std::unique_ptr<base::OneShotTimer> timer) {
offline_signin_limit_timer_ = std::move(timer);
}

void SAMLOfflineSigninLimiter::Shutdown() {
void OfflineSigninLimiter::Shutdown() {
offline_signin_limit_timer_->Stop();
pref_change_registrar_.RemoveAll();
}

void SAMLOfflineSigninLimiter::OnResume() {
void OfflineSigninLimiter::OnResume() {
UpdateLimit();
}

void SAMLOfflineSigninLimiter::OnSessionStateChanged() {
void OfflineSigninLimiter::OnSessionStateChanged() {
if (!session_manager::SessionManager::Get()->IsScreenLocked()) {
UpdateLimit();
}
}

SAMLOfflineSigninLimiter::SAMLOfflineSigninLimiter(Profile* profile,
base::Clock* clock)
OfflineSigninLimiter::OfflineSigninLimiter(Profile* profile, base::Clock* clock)
: profile_(profile),
clock_(clock ? clock : base::DefaultClock::GetInstance()),
offline_signin_limit_timer_(std::make_unique<base::OneShotTimer>()) {}

SAMLOfflineSigninLimiter::~SAMLOfflineSigninLimiter() {
OfflineSigninLimiter::~OfflineSigninLimiter() {
base::PowerMonitor::RemoveObserver(this);
auto* session_manager = session_manager::SessionManager::Get();
if (session_manager) {
session_manager->RemoveObserver(this);
}
}

void SAMLOfflineSigninLimiter::UpdateLimit() {
void OfflineSigninLimiter::UpdateLimit() {
// Stop the `offline_signin_limit_timer_`.
offline_signin_limit_timer_->Stop();

Expand Down Expand Up @@ -204,10 +203,10 @@ void SAMLOfflineSigninLimiter::UpdateLimit() {
// `OneShotTimer`.
offline_signin_limit_timer_->Start(
FROM_HERE, offline_signin_time_limit - time_since_last_gaia_signin, this,
&SAMLOfflineSigninLimiter::ForceOnlineLogin);
&OfflineSigninLimiter::ForceOnlineLogin);
}

void SAMLOfflineSigninLimiter::ForceOnlineLogin() {
void OfflineSigninLimiter::ForceOnlineLogin() {
const user_manager::User* user =
ProfileHelper::Get()->GetUserByProfile(profile_);
DCHECK(user);
Expand All @@ -228,7 +227,7 @@ void SAMLOfflineSigninLimiter::ForceOnlineLogin() {
offline_signin_limit_timer_->Stop();
}

void SAMLOfflineSigninLimiter::UpdateOnlineSigninData(
void OfflineSigninLimiter::UpdateOnlineSigninData(
base::Time time,
base::Optional<base::TimeDelta> limit) {
const user_manager::User* user =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SAML_SAML_OFFLINE_SIGNIN_LIMITER_H_
#define CHROME_BROWSER_CHROMEOS_LOGIN_SAML_SAML_OFFLINE_SIGNIN_LIMITER_H_
#ifndef CHROME_BROWSER_CHROMEOS_LOGIN_SIGNIN_OFFLINE_SIGNIN_LIMITER_H_
#define CHROME_BROWSER_CHROMEOS_LOGIN_SIGNIN_OFFLINE_SIGNIN_LIMITER_H_

#include <memory>

Expand All @@ -28,12 +28,9 @@ namespace chromeos {
// Gaia without SAML or with SAML can use offline authentication against a
// cached password before being forced to go through online authentication
// against GAIA again.
// TODO(b/179637649): Change `SAMLOfflineSigninLimiter` class name since the
// class becomes applied to non-SAML users too.
class SAMLOfflineSigninLimiter
: public KeyedService,
public base::PowerObserver,
public session_manager::SessionManagerObserver {
class OfflineSigninLimiter : public KeyedService,
public base::PowerObserver,
public session_manager::SessionManagerObserver {
public:
// Called when the user successfully authenticates. `auth_flow` indicates
// the type of authentication flow that the user went through.
Expand All @@ -52,13 +49,13 @@ class SAMLOfflineSigninLimiter
void OnSessionStateChanged() override;

private:
friend class SAMLOfflineSigninLimiterFactory;
friend class SAMLOfflineSigninLimiterTest;
friend class OfflineSigninLimiterFactory;
friend class OfflineSigninLimiterTest;

// `profile` and `clock` must remain valid until Shutdown() is called. If
// `clock` is NULL, the shared base::DefaultClock instance will be used.
SAMLOfflineSigninLimiter(Profile* profile, base::Clock* clock);
~SAMLOfflineSigninLimiter() override;
OfflineSigninLimiter(Profile* profile, base::Clock* clock);
~OfflineSigninLimiter() override;

// Recalculates the amount of time remaining until online login should be
// forced and sets the `offline_signin_limit_timer_` accordingly. If the limit
Expand All @@ -80,9 +77,9 @@ class SAMLOfflineSigninLimiter

std::unique_ptr<base::OneShotTimer> offline_signin_limit_timer_;

DISALLOW_COPY_AND_ASSIGN(SAMLOfflineSigninLimiter);
DISALLOW_COPY_AND_ASSIGN(OfflineSigninLimiter);
};

} // namespace chromeos

#endif // CHROME_BROWSER_CHROMEOS_LOGIN_SAML_SAML_OFFLINE_SIGNIN_LIMITER_H_
#endif // CHROME_BROWSER_CHROMEOS_LOGIN_SIGNIN_OFFLINE_SIGNIN_LIMITER_H_
Loading

0 comments on commit a516967

Please sign in to comment.