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

Update Security Samples #301

Closed
wants to merge 14 commits into from
Closed

Update Security Samples #301

wants to merge 14 commits into from

Conversation

TimHess
Copy link
Member

@TimHess TimHess commented Jul 14, 2023

Use minimal API, add actuators, only listen over HTTPS, improve usability

TODO: update integration test, help users understand how to enable UAA as a user store for the tile

Security/src/CloudFoundrySingleSignon/Program.cs Outdated Show resolved Hide resolved
Security/src/CloudFoundrySingleSignon/README-UAA.md Outdated Show resolved Hide resolved
After authenticating, add a new `user` and `group` to the UAA Server database. Do NOT change the group name: `testgroup` as it is used for policy based authorization in the sample application. Feel free to change the username and password to anything you would like.

1. uaac group add testgroup
1. uaac user add dave --given_name Dave --family_name Tillman --emails dave@testcloud.com --password Password1!
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps replace 'dave' with something else? (multiple places in this file and others)


### Add User-Provided Service with OAuth Details

Last, we will create a user-provide service that includes the appropriate UAA server configuration data. Use the sample below to pass the parameters directly to the `cf cups` command, replacing `<YOUR-CLOUDFOUNDRY-SYSTEM-DOMAIN>` with your domain.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Last, we will create a user-provide service that includes the appropriate UAA server configuration data. Use the sample below to pass the parameters directly to the `cf cups` command, replacing `<YOUR-CLOUDFOUNDRY-SYSTEM-DOMAIN>` with your domain.
Last, we will create a user-provided service that includes the appropriate UAA server configuration data. Use the sample below to pass the parameters directly to the `cf cups` command, replacing `<YOUR-CLOUDFOUNDRY-SYSTEM-DOMAIN>` with your domain.

Security/src/CloudFoundrySingleSignon/README-UAA.md Outdated Show resolved Hide resolved
* `cf push -f manifest.yml -p bin/Debug/netcoreapp3.1/linux-x64/publish`
* `cf push -f manifest-windows.yml -p bin/Debug/netcoreapp3.1/win10-x64/publish`

> Note: The provided manifest(s) will create an app named `single-signon` and attempt to bind it to the user-provided service `myOAuthService`.
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this was renamed to mySSOService in other places. Should this be renamed accordingly?

Suggested change
> Note: The provided manifest(s) will create an app named `single-signon` and attempt to bind it to the user-provided service `myOAuthService`.
> Note: The provided manifest(s) will create an app named `single-signon` and attempt to bind it to the user-provided service `mySSOService`.

Comment on lines 38 to 39
var options = serviceProvider.GetService<IOptions<CertificateOptions>>();
if (options?.Value.Certificate != null)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think GetService for an IOptions<> ever returns null. If there's no configuration, you'll get a default instance.

Suggested change
var options = serviceProvider.GetService<IOptions<CertificateOptions>>();
if (options?.Value.Certificate != null)
var options = serviceProvider.GetRequiredService<IOptions<CertificateOptions>>();
if (options.Value.Certificate != null)

The same suggestion applies to other places in this PR, which I haven't marked.

options.AddPolicy("testgroup1", policy => policy.RequireClaim("scope", "testgroup1"));
});

// Add Redis to allow scaling beyond a single instance
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Add Redis to allow scaling beyond a single instance
// Uncomment the code below to add Redis to allow scaling beyond a single instance

}
else
{
serviceHostname = serviceName + hostName.Substring(indx);
serviceHostname = string.Concat(serviceName, hostName.AsSpan(indexOfHost));
Copy link
Member

Choose a reason for hiding this comment

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

Just out of curiosity, did you get this suggestion from an analyzer? Resharper suggests using range indexer syntax.

Suggested change
serviceHostname = string.Concat(serviceName, hostName.AsSpan(indexOfHost));
serviceHostname = $"{serviceName}{hostName[indx..]}";

Copy link
Member Author

Choose a reason for hiding this comment

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

This was probably a Visual Studio suggestion, Resharper's preference is fine with me

ccheetham and others added 14 commits June 6, 2024 16:53
Bumps [MongoDB.Driver](https://github.com/mongodb/mongo-csharp-driver) from 2.18.0 to 2.19.0.
- [Release notes](https://github.com/mongodb/mongo-csharp-driver/releases)
- [Commits](mongodb/mongo-csharp-driver@v2.18.0...v2.19.0)

---
updated-dependencies:
- dependency-name: MongoDB.Driver
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [requests](https://github.com/psf/requests) from 2.28.2 to 2.31.0.
- [Release notes](https://github.com/psf/requests/releases)
- [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md)
- [Commits](psf/requests@v2.28.2...v2.31.0)

---
updated-dependencies:
- dependency-name: requests
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
See https://hub.docker.com/_/consul:
> Upcoming in Consul 1.16, we will stop publishing official Dockerhub images and publish only our Verified Publisher images. Users of Docker images should pull from hashicorp/consul instead of consul. Verified Publisher images can be found at https://hub.docker.com/r/hashicorp/consul.
Bumps [mechanicalsoup](https://github.com/MechanicalSoup/MechanicalSoup) from 1.2.0 to 1.3.0.
- [Release notes](https://github.com/MechanicalSoup/MechanicalSoup/releases)
- [Changelog](https://github.com/MechanicalSoup/MechanicalSoup/blob/main/docs/ChangeLog.rst)
- [Commits](MechanicalSoup/MechanicalSoup@v1.2.0...v1.3.0)

---
updated-dependencies:
- dependency-name: mechanicalsoup
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.0.2 to 2.0.6.
- [Release notes](https://github.com/urllib3/urllib3/releases)
- [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst)
- [Commits](urllib3/urllib3@2.0.2...2.0.6)

---
updated-dependencies:
- dependency-name: urllib3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.0.6 to 2.0.7.
- [Release notes](https://github.com/urllib3/urllib3/releases)
- [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst)
- [Commits](urllib3/urllib3@2.0.6...2.0.7)

---
updated-dependencies:
- dependency-name: urllib3
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [Azure.Identity](https://github.com/Azure/azure-sdk-for-net) from 1.5.0 to 1.10.2.
- [Release notes](https://github.com/Azure/azure-sdk-for-net/releases)
- [Commits](Azure/azure-sdk-for-net@Azure.Identity_1.5.0...Azure.Identity_1.10.2)

---
updated-dependencies:
- dependency-name: Azure.Identity
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [Azure.Identity](https://github.com/Azure/azure-sdk-for-net) from 1.5.0 to 1.10.2.
- [Release notes](https://github.com/Azure/azure-sdk-for-net/releases)
- [Commits](Azure/azure-sdk-for-net@Azure.Identity_1.5.0...Azure.Identity_1.10.2)

---
updated-dependencies:
- dependency-name: Azure.Identity
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [Azure.Identity](https://github.com/Azure/azure-sdk-for-net) from 1.5.0 to 1.10.2.
- [Release notes](https://github.com/Azure/azure-sdk-for-net/releases)
- [Commits](Azure/azure-sdk-for-net@Azure.Identity_1.5.0...Azure.Identity_1.10.2)

---
updated-dependencies:
- dependency-name: Azure.Identity
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [Azure.Identity](https://github.com/Azure/azure-sdk-for-net) from 1.5.0 to 1.10.2.
- [Release notes](https://github.com/Azure/azure-sdk-for-net/releases)
- [Commits](Azure/azure-sdk-for-net@Azure.Identity_1.5.0...Azure.Identity_1.10.2)

---
updated-dependencies:
- dependency-name: Azure.Identity
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Use minimal API, add actuators, only listen over HTTPS, improve usability
@TimHess
Copy link
Member Author

TimHess commented Jun 6, 2024

I will reopen a new PR, this history is probably not worth saving

@TimHess TimHess closed this Jun 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants