Add new provider - SAML via Azure AD #169
Conversation
greenpau
changed the title
greenpau
force-pushed
the
samlprovider
branch
8 times, most recently
from
3acaba5 to
1a72c1d
Compare
greenpau
force-pushed
the
samlprovider
branch
2 times, most recently
from
cee0b9a to
073aeec
Compare
|
Hello @greenpau, thank you for your contribution! For now simply put the documentation in the README.md. |
greenpau
force-pushed
the
samlprovider
branch
11 times, most recently
from
fb49bf7 to
ba80425
Compare
|
@g-w , could you please review the addition. I know that the coverage is not there. I would like to get general agreement on the proposal. Once finalized, I will improve the coverage. |
greenpau
force-pushed
the
samlprovider
branch
from
ba80425 to
41b7183
Compare
greenpau
changed the title
|
@g-w , any feedback? |
|
Hello @greenpau, sorry for the delays, but since we are currently not using azure and i am not familiar with it, hence reviewien the changes is a bigger task. I will come back to in the next days. |
There was a problem hiding this comment.
Hello @greenpau,
I had a look at your changes. Currently there are some problems with your changes that have to be fixed.
- The functionality should work without caddy
- Azure SAML should work like the other authentication backends, ie should follow the configuration style
- There are (almost) no tests that assure the correctness of the implementation
Please have a look into httpasswd backend for an example how the backend mechanism works. If you have any further questions, please come back to me.
| login { | ||
| success_url /private | ||
|
|
||
| azure_enabled true |
There was a problem hiding this comment.
Please use the configuration scheme as the other methods. It should read something like:
azure_saml metadata_location=/... idp_sign_cert_location=/...
|
|
||
| ### Azure AD Applications | ||
|
|
||
| #### Caddy Configuration |
There was a problem hiding this comment.
Please put documentation on caddy specific Configuration in /caddy/README.md and loginsrv standalone configuration in the /README.md.
|
|
||
| #### Set Up Azure AD Application | ||
|
|
||
| In Azure AD, you will have an application, e.g. "My Infosec". |
There was a problem hiding this comment.
I think this section should be shorter and assume general knowledge on azure saml.
| if config.Azure == nil { | ||
| return nil, errors.New(errMsg) | ||
| } | ||
| if !config.Azure.Enabled { |
There was a problem hiding this comment.
Azure SAML should be a login backend.
|
moved it to a new caddy v2 module https://github.com/greenpau/caddy-auth-forms |
Resolves: #168