Skip to content

Add patch for client credential support to cpprest package.#443

Closed
abeltrano wants to merge 4 commits intomicrosoft:1.0-devfrom
abeltrano:anbeltra/cpprest_clientcreds
Closed

Add patch for client credential support to cpprest package.#443
abeltrano wants to merge 4 commits intomicrosoft:1.0-devfrom
abeltrano:anbeltra/cpprest_clientcreds

Conversation

@abeltrano
Copy link
Copy Markdown

@abeltrano abeltrano commented Dec 8, 2020

Merge Checklist

All boxes should be checked before merging the PR (just tick any boxes which don't apply to this PR)

  • The toolchain has been rebuilt successfully (or no changes were made to it)
  • The toolchain/worker package manifests are up-to-date
  • Any updated packages successfully build (or no packages were changed)
  • All package sources are available
  • cgmanifest files are up-to-date and sorted (./cgmanifest.json, ./toolkit/tools/cgmanifest.json, ./toolkit/scripts/toolchain/cgmanifest.json)
  • LICENSE-MAP files are up-to-date (./SPECS/LICENSES-AND-NOTICES/LICENSES-MAP.md, ./SPECS/LICENSES-AND-NOTICES/LICENSE-EXCEPTIONS.PHOTON)
  • All source files have up-to-date hashes in the *.signatures.json files
  • sudo make go-tidy-all and sudo make go-test-coverage pass
  • Documentation has been updated to match any changes to the build system
  • Ready to merge
  • NOTE: I could not get the build-packages make target to work, so instead, rebuilt this package using rpmbuild directly.

Summary

Add support for OAuth2 client credentials grant type to the cpprest package. This grant type is commonly used in the absence of a user, so is useful for applications involving IoT devices.

Change Log
  • Add cpprest-2.10.14-Add-support-for-oauth2-using-client-credentials.patch, enabling grant type 'client_credentials'.
Does this affect the toolchain?

NO

Associated issues
Test Methodology
  • Local package build of cpprestsdk package.
  • This patch has been used successfully since 09/15/2020 in a Mariner-derivative at Microsoft for an Azure project, using the same base version of cppresksdk (2.10.14). The package is used by two other packages in the derivative.

@ghost ghost added the Packaging label Dec 8, 2020
@abeltrano abeltrano marked this pull request as ready for review December 8, 2020 23:20
Comment thread SPECS/cpprest/cpprest.spec Outdated
Copy link
Copy Markdown
Author

@abeltrano abeltrano Dec 8, 2020

Choose a reason for hiding this comment

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

spec-lint checks required this change, and the others where existing lines in this file were moved.

@mrgirgin mrgirgin requested a review from jslobodzian December 12, 2020 01:36
@abeltrano abeltrano force-pushed the anbeltra/cpprest_clientcreds branch from 390793a to 25fea6a Compare December 15, 2020 00:55
DAT(state, "state")
diff --git a/Release/include/cpprest/oauth2.h b/Release/include/cpprest/oauth2.h
index 693ebbe3..68a7c7b9 100644
--- a/Release/include/cpprest/oauth2.h
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is this a public API change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This particular change to http_constants.dat is part of a private header (include/cpprest/details), so no. This adds a const std::string to a private helper class, as below (note the details namespace):

/// oAuth 2.0 library.
namespace oauth2
{
namespace details
{
class oauth2_handler;

// Constant strings for OAuth 2.0.
typedef utility::string_t oauth2_string;
class oauth2_strings
{
public:
#define _OAUTH2_STRINGS
#define DAT(a_, b_) _ASYNCRTIMP static const oauth2_string a_;
#include "cpprest/details/http_constants.dat"
#undef _OAUTH2_STRINGS
#undef DAT
};

} // namespace details

To add some more color, the added constant client_credentials is used in the impl of a newly added public function token_from_client_credentials() to add a uri parameter requiring a specific name for the oauth2 protocol. Since this is a new function that must be called explicitly, there should be no way for it to cause back-compat issues.

@abeltrano abeltrano closed this Dec 15, 2020
@abeltrano
Copy link
Copy Markdown
Author

After discussion with @jslobodzian, we'll rescind this PR until the change is accepted in the upstream cpprest repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants