Skip to content

Commit

Permalink
[OAuth2Manager] Remove APIs for Implicit grant type (#4979)
Browse files Browse the repository at this point in the history 
* [OAuth2Manager] Remove APIs for Implicit grant type

* Remove implicit refernces

* Telemetry

* Add security recommendations to OAuth2Manager spec.
  • Loading branch information
@akanpatel2206
akanpatel2206 authored Dec 23, 2024
1 parent d2b26bb commit 18f3a27
Show file tree
Hide file tree
Showing 9 changed files with 15 additions and 342 deletions.
52 changes: 0 additions & 52 deletions dev/OAuth/AuthRequestAsyncOperation.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,58 +13,6 @@ using namespace winrt::Windows::Foundation;
using namespace winrt::Windows::Foundation::Collections;
using namespace winrt::Windows::Security::Cryptography;

AuthRequestAsyncOperation::AuthRequestAsyncOperation(winrt::hstring& state)
{
try
{
if (state.empty())
{
while (true)
{
state = random_base64urlencoded_string(32);
if (try_create_pipe(state))
{
break;
}

// 'FILE_FLAG_FIRST_PIPE_INSTANCE' is documented as failing with 'ERROR_ACCESS_DENIED' if a pipe
// with the same name has already been created.
if (auto err = ::GetLastError(); err != ERROR_ACCESS_DENIED)
{
throw winrt::hresult_error(HRESULT_FROM_WIN32(err),
L"Generation of a unique state value unexpectedly failed");
}
}
}
else if (!try_create_pipe(state))
{
auto err = ::GetLastError();
auto msg =
(err == ERROR_ACCESS_DENIED) ? L"Provided state value is not unique" : L"Failed to create named pipe";
throw winrt::hresult_error(HRESULT_FROM_WIN32(err), msg);
}

m_overlapped.hEvent = ::CreateEventW(nullptr, true, false, nullptr);
if (!m_overlapped.hEvent)
{
throw winrt::hresult_error(HRESULT_FROM_WIN32(::GetLastError()), L"Failed to create an event");
}

m_ptp.reset(::CreateThreadpoolWait(async_callback, this, nullptr)); // Use reset() to initialize
if (!m_ptp)
{
throw winrt::hresult_error(HRESULT_FROM_WIN32(::GetLastError()), L"Failed to create threadpool wait");
}
connect_to_new_client();
}
catch (...)
{
// Throwing in a constructor will cause the destructor not to run...
destroy();
throw;
}
}

AuthRequestAsyncOperation::AuthRequestAsyncOperation(implementation::AuthRequestParams* params) :
m_params(params->get_strong())
{
Expand Down
1 change: 0 additions & 1 deletion dev/OAuth/AuthRequestAsyncOperation.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ struct AuthRequestAsyncOperation :
winrt::implements<AuthRequestAsyncOperation, foundation::IAsyncOperation<oauth::AuthRequestResult>,
foundation::IAsyncInfo>
{
AuthRequestAsyncOperation(winrt::hstring& state);
AuthRequestAsyncOperation(oauth::implementation::AuthRequestParams* params);
~AuthRequestAsyncOperation();

Expand Down
49 changes: 6 additions & 43 deletions dev/OAuth/OAuth.idl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,6 @@ namespace Microsoft.Security.Authentication.OAuth
[contract(OAuthContract, 1), feature(Feature_OAuth)]
static runtimeclass OAuth2Manager
{
// Initiates an authorization request in the user's default browser as described by RFC 6749 section 3.1. The
// returned 'IAsyncOperation' will remain in the 'Started' state until it is either cancelled or completed by a
// call to 'CompleteAuthRequest'. This performs authorization of response_type="token".
static Windows.Foundation.IAsyncOperation<AuthRequestResult> RequestAuthAsync(
Microsoft.UI.WindowId parentWindowId,
Windows.Foundation.Uri completeAuthEndpoint,
Windows.Foundation.Uri redirectUri);

// Initiates an authorization request in the user's default browser as described by RFC 6749 section 3.1. The
// returned 'IAsyncOperation' will remain in the 'Started' state until it is either cancelled or completed by a
// call to 'CompleteAuthRequest'.This performs authorization of response_type="token".
static Windows.Foundation.IAsyncOperation<AuthRequestResult> RequestAuthAsync(
Microsoft.UI.WindowId parentWindowId,
Windows.Foundation.Uri completeAuthEndpoint);

// Initiates an authorization request in the user's default browser as described by RFC 6749 section 3.1. The
// returned 'IAsyncOperation' will remain in the 'Started' state until it is either cancelled or completed by a
// call to 'CompleteAuthRequest'.
Expand Down Expand Up @@ -98,16 +83,8 @@ namespace Microsoft.Security.Authentication.OAuth
// parameters as well as a redirect URI, which is frequently specified.
static AuthRequestParams CreateForAuthorizationCodeRequest(String clientId, Windows.Foundation.Uri redirectUri);

// Helper method to create for an implicit grant request ("token" response type) with required parameters, per
// RFC 6749 section 4.2.1.
static AuthRequestParams CreateForImplicitRequest(String clientId);
// Helper method to create for an implicit grant request ("token" response type) with required parameters as
// well as a redirect URI, which is frequently specified.
static AuthRequestParams CreateForImplicitRequest(String clientId, Windows.Foundation.Uri redirectUri);

// Specifies the required "response_type" parameter of the authorization request. This property is initialized
// by the creation function used ("code" for 'CreateForAuthorizationCodeRequest' and "token" for
// 'CreateForImplicitRequest').
// by the creation function used ("code" for 'CreateForAuthorizationCodeRequest').
//
// Defined by RFC 6749: The OAuth 2.0 Authorization Framework, sections 4.1.1 and 4.2.1
// https://www.rfc-editor.org/rfc/rfc6749#section-4.1.1
Expand Down Expand Up @@ -157,9 +134,7 @@ namespace Microsoft.Security.Authentication.OAuth
String CodeChallenge{ get; set; };

// Specifies the optional "code_challenge_method" parameter of the authorization request. For authorization code
// requests, this value defaults to 'S256'. For implicit requests, this value defaults to 'None' and cannot be
// changed.
//
// requests, this value defaults to 'S256'.
// Defined by RFC 7636: Proof Key for Code Exchange by OAuth Public Clients, section 4.3
// https://www.rfc-editor.org/rfc/rfc7636#section-4.3
CodeChallengeMethodKind CodeChallengeMethod { get; set; };
Expand All @@ -186,30 +161,22 @@ namespace Microsoft.Security.Authentication.OAuth
// https://www.rfc-editor.org/rfc/rfc6749#section-4.1.2
String Code { get; };

// From the "access_token" parameter of the authorization response. Set only if the request was an implicit
// request.
//
// From the "access_token" parameter of the authorization response.
// Defined by RFC 6749: The OAuth 2.0 Authorization Framework, section 4.2.2
// https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2
String AccessToken { get; };

// From the "token_type" parameter of the authorization response. Set only if the request was an implicit
// request.
//
// From the "token_type" parameter of the authorization response.
// Defined by RFC 6749: The OAuth 2.0 Authorization Framework, section 4.2.2
// https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2
String TokenType { get; };

// From the "expires_in" parameter of the authorization response. An optional parameter that may be set only if
// the request was an implicit request.
//
// From the "expires_in" parameter of the authorization response.
// Defined by RFC 6749: The OAuth 2.0 Authorization Framework, section 4.2.2
// https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2
String ExpiresIn { get; }; // TODO: DateTime?

// From the "scope" parameter of the authorization response. An optional parameter that may be set only if the
// request was an implicit request.
//
// From the "scope" parameter of the authorization response.
// Defined by RFC 6749: The OAuth 2.0 Authorization Framework, section 4.2.2
// https://www.rfc-editor.org/rfc/rfc6749#section-4.2.2
String Scope { get; };
Expand Down Expand Up @@ -281,10 +248,6 @@ namespace Microsoft.Security.Authentication.OAuth
// 4.1.3.
static TokenRequestParams CreateForAuthorizationCodeRequest(AuthResponse authResponse);

// Helper method to create for a resource owner password credentials grant request ("password" grant type),
// initialized with the required parameters, per RFC 6749 section 4.3.2.
static TokenRequestParams CreateForResourceOwnerPasswordCredentials(String username, String password);

// Helper method to create for a client credentials grant request ("client_credentials" grant type), initialized
// with the required parameters, per RFC 6749 section 4.4.2.
static TokenRequestParams CreateForClientCredentials();
Expand Down
92 changes: 0 additions & 92 deletions dev/OAuth/OAuth2Manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,74 +23,6 @@ using namespace winrt::Windows::Web::Http;

namespace winrt::Microsoft::Security::Authentication::OAuth::factory_implementation
{
IAsyncOperation<AuthRequestResult> OAuth2Manager::RequestAuthAsync(winrt::Microsoft::UI::WindowId const& parentWindowId,
const Uri& completeAuthEndpoint,
const Uri& redirectUri)
{
THROW_HR_IF(E_NOTIMPL, !::Microsoft::Security::Authentication::OAuth::Feature_OAuth::IsEnabled());

bool isAppPackaged = m_telemetryHelper.IsPackagedApp();
PCWSTR appName = m_telemetryHelper.GetAppName().c_str();
OAuth2ManagerTelemetry::RequestAuthAsyncTriggered(isAppPackaged, appName, true);

winrt::hstring state;
auto asyncOp = winrt::make_self<AuthRequestAsyncOperation>(state);

{
std::lock_guard guard{ m_mutex };
m_pendingAuthRequests.push_back(AuthRequestState{ state, asyncOp });
}

try
{
// Pipe server has been successfully set up. Initiate the launch
auto url = create_implicit_url(completeAuthEndpoint, state, redirectUri);

// Launch browser
execute_shell(parentWindowId, url);
}
catch (...)
{
try_remove(asyncOp.get());
throw;
}

return *asyncOp;
}

IAsyncOperation<AuthRequestResult> OAuth2Manager::RequestAuthAsync(winrt::Microsoft::UI::WindowId const& parentWindowId,
const Uri& completeAuthEndpoint)
{
THROW_HR_IF(E_NOTIMPL, !::Microsoft::Security::Authentication::OAuth::Feature_OAuth::IsEnabled());


bool isAppPackaged = m_telemetryHelper.IsPackagedApp();
PCWSTR appName = m_telemetryHelper.GetAppName().c_str();
OAuth2ManagerTelemetry::RequestAuthAsyncTriggered(isAppPackaged, appName, false);

winrt::hstring state;
auto asyncOp = winrt::make_self<AuthRequestAsyncOperation>(state);

{
std::lock_guard guard{ m_mutex };
m_pendingAuthRequests.push_back(AuthRequestState{ state, asyncOp });
}

try
{
// Pipe server has been successfully set up. Initiate the launch
auto url = create_implicit_url(completeAuthEndpoint, state, nullptr);

// Launch browser
execute_shell(parentWindowId, url);
}
catch (...)
{
try_remove(asyncOp.get());
throw;
}
return *asyncOp;
}

IAsyncOperation<AuthRequestResult> OAuth2Manager::RequestAuthWithParamsAsync(winrt::Microsoft::UI::WindowId const& parentWindowId,
const Uri& authEndpoint,
Expand Down Expand Up @@ -408,30 +340,6 @@ namespace winrt::Microsoft::Security::Authentication::OAuth::factory_implementat
return result;
}

std::wstring OAuth2Manager::create_implicit_url(const foundation::Uri& completeAuthEndpoint, const winrt::hstring& state, const foundation::Uri& redirectUri)
{
std::lock_guard guard{ m_mutex };
// Per RFC 6749 section 3.1, the auth endpoint URI *MAY* contain a query string, which must be retained
std::wstring result{ completeAuthEndpoint.RawUri() };
if (completeAuthEndpoint.Query().empty())
{
result += L"?state=";
}
else
{
result += L"&state=";
}
result += Uri::EscapeComponent(state);
result += L"&response_type=token";

if (redirectUri)
{
result += L"&redirect_uri=";
result += Uri::EscapeComponent(redirectUri.RawUri());
}
return result;
}

void OAuth2Manager::execute_shell(winrt::Microsoft::UI::WindowId const& parentWindowId, const std::wstring& url)
{
// Convert parentWindowId to HWND
Expand Down
21 changes: 0 additions & 21 deletions dev/OAuth/OAuth2Manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,6 @@ namespace winrt::Microsoft::Security::Authentication::OAuth::factory_implementat

struct OAuth2Manager : OAuth2ManagerT<OAuth2Manager, implementation::OAuth2Manager, winrt::static_lifetime>
{
foundation::IAsyncOperation<oauth::AuthRequestResult> RequestAuthAsync(
winrt::Microsoft::UI::WindowId const& parentWindowId, const foundation::Uri& completeAuthEndpoint, const foundation::Uri& redirectUri);
foundation::IAsyncOperation<oauth::AuthRequestResult> RequestAuthAsync(
winrt::Microsoft::UI::WindowId const& parentWindowId, const foundation::Uri& completeAuthEndpoint);
foundation::IAsyncOperation<oauth::AuthRequestResult> RequestAuthWithParamsAsync(
winrt::Microsoft::UI::WindowId const& parentWindowId, const foundation::Uri& authEndpoint, const oauth::AuthRequestParams& params);
bool CompleteAuthRequest(const foundation::Uri& responseUri);
Expand All @@ -41,7 +37,6 @@ namespace winrt::Microsoft::Security::Authentication::OAuth::factory_implementat
private:
AuthRequestState try_remove(AuthRequestAsyncOperation* op);

std::wstring create_implicit_url(const foundation::Uri& completeAuthEndpoint, const winrt::hstring& state, const foundation::Uri& redirectUri);
void execute_shell(winrt::Microsoft::UI::WindowId const& parentWindowId, const std::wstring& url);
std::shared_mutex m_mutex;
TelemetryHelper m_telemetryHelper;
Expand All @@ -53,22 +48,6 @@ namespace winrt::Microsoft::Security::Authentication::OAuth::implementation
{
struct OAuth2Manager
{
static foundation::IAsyncOperation<oauth::AuthRequestResult> RequestAuthAsync(
winrt::Microsoft::UI::WindowId const& parentWindowId,
foundation::Uri completeAuthEndpoint, foundation::Uri redirectUri)
{
return winrt::make_self<factory_implementation::OAuth2Manager>()->RequestAuthAsync(parentWindowId,
completeAuthEndpoint,
redirectUri);
}

static foundation::IAsyncOperation<oauth::AuthRequestResult> RequestAuthAsync(
winrt::Microsoft::UI::WindowId const& parentWindowId,
foundation::Uri completeAuthEndpoint)
{
return winrt::make_self<factory_implementation::OAuth2Manager>()->RequestAuthAsync(parentWindowId,
completeAuthEndpoint);
}

static foundation::IAsyncOperation<oauth::AuthRequestResult> RequestAuthWithParamsAsync(
winrt::Microsoft::UI::WindowId const& parentWindowId,
Expand Down
2 changes: 0 additions & 2 deletions dev/OAuth/OAuth2ManagerTelemetry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ class OAuth2ManagerTelemetry : public wil::TraceLoggingProvider
(0x27d8ee3f, 0xd704, 0x45d6, 0xb6, 0x6c, 0x1d, 0xad, 0x95, 0x79, 0x5c, 0xe5));
//{27d8ee3f-d704-45d6-b66c-1dad95795ce5}
public:
DEFINE_COMPLIANT_MEASURES_EVENT_PARAM3(RequestAuthAsyncTriggered, PDT_ProductAndServicePerformance,
bool, IsAppPackaged, PCWSTR, AppName, bool, IsRedirectURIPassed);

DEFINE_COMPLIANT_MEASURES_EVENT_PARAM3(RequestAuthWithParamsAsyncTriggered, PDT_ProductAndServicePerformance,
bool, IsAppPackaged, PCWSTR, AppName, PCWSTR, ResponseType);
Expand Down
10 changes: 0 additions & 10 deletions dev/OAuth/TokenRequestParams.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,6 @@ namespace winrt::Microsoft::Security::Authentication::OAuth::implementation
return *result;
}

oauth::TokenRequestParams TokenRequestParams::CreateForResourceOwnerPasswordCredentials(
const winrt::hstring& username, const winrt::hstring& password)
{
auto result = winrt::make_self<TokenRequestParams>(L"password");
result->m_username = username;
result->m_password = password;

return *result;
}

oauth::TokenRequestParams TokenRequestParams::CreateForClientCredentials()
{
return winrt::make<TokenRequestParams>(L"client_credentials");
Expand Down
2 changes: 0 additions & 2 deletions dev/OAuth/TokenRequestParams.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ namespace winrt::Microsoft::Security::Authentication::OAuth::implementation
TokenRequestParams(const winrt::hstring& grantType);

static oauth::TokenRequestParams CreateForAuthorizationCodeRequest(const oauth::AuthResponse& authResponse);
static oauth::TokenRequestParams CreateForResourceOwnerPasswordCredentials(const winrt::hstring& username,
const winrt::hstring& password);
static oauth::TokenRequestParams CreateForClientCredentials();
static oauth::TokenRequestParams CreateForExtension(const foundation::Uri& extensionUri);
static oauth::TokenRequestParams CreateForRefreshToken(const winrt::hstring& refreshToken);
Expand Down
Loading

0 comments on commit 18f3a27

Please sign in to comment.