Hey, we’d like some help with our PR on saml feder...
# contributors
f
Hey, we’d like some help with our PR on saml federation in Kratos. We hadn't updated kratos-selfservice-ui-node for a while and were still on the pink interface. Our SAML implementation is going well, and we had our own "Sign in with SAML" button. I should point out that at no time did we need to modify the ui-node code to have the button. Since we updated the ui-node on the last commit (we update regularly on Kratos too), we lost our SAML button, only the OIDC is here now. We implemented a PopulateLoginMethod method for SAML as well as you did for OIDC (strategy_login.go) and the populateMethod method (strategy.go) We also have our own AddProviders and AddProvider methods for SAML (types.go) in order to append the corresponding node to the container. At the end, the method NewInfoLoginWith (message_loging.go) is called for both OIDC and SAML. So could you give us a clue about why out “Sign in with SAML” button doesn’t appear anymore please? Have there been any changes, either on the kratos side or on the ui side that make it not enough for the button to appear anymore? We can't find it on our side. Thank you!
👍 1
h
can you point me to the line of code where you populate the form?
h
Ah yes, I think the
node.SAMLGroup
looks new. It’s probably being filtered out. @proud-plumber-24205 is back next week - he’s the author of Ory elements - and can take a look
f
We added
node.SAMLGroup
as a second step to return user groups in our SAML assertions, but it is normally decorrelated from the login button. Thanks for your answer we'll wait for the answer of @proud-plumber-24205 :)
p
Hi @faint-sandwich-26722 I'm guessing we would have a "Sign in with SAML" at the top with the OIDC buttons? I guess we would need to add a new "section" to elements like we have with OIDC here. Then just check if the flow contains this specified node group and display it above the
oidc
section here You will probably also need to add a new settings section for the settings page (link / unlick saml?) like we have it here.
for the settings page we use the
UserSettingsCard
component which renders the sections the flow contains https://github.com/ory/elements/blob/main/src/react-components/ory/user-settings-card.tsx
f
Thank you for your answer @proud-plumber-24205 Yes, the SAML button is supposed to be on top of the OIDC one, but only if the SAML provider has been configured in the kratos.yml of kratos. I'm not sure I understand: before, we only needed to modify kratos for the ui to update itself via our kratos.yml configuration (via the populate method in particular). Now it depends on ory/elements too? Does it mean that we also should contribute on this repository as part of our first PR on kratos for SAML?
p
Hi @faint-sandwich-26722 Yes, essentially we moved all of the UI rendering and handling of the flow object to Ory Elements so that it is reusable across many examples as well as the kratos-selfservice-ui-node repository. Before we just rendered whatever kratos gave us inside the
flow.ui
object which is very dynamic, but this causes problems with structure and design. So right now we opted to expect certain flow nodes in the structure we want. Later we will probably abstract this more so that anyone can change this structure without needing to change the component code. But for now let's add a new section above the
oidc
section inside the
UserAuthCard
and
UserSettingsCard
f
Okay thank you very much for your reactivity, I'll keep you informed 🙂
❤️ 1
Hey @proud-plumber-24205 here's some news: what you told me about the section in ory/elements was indeed what I was looking for. Please expect a PR in this repo connected to our current PR in ory/kratos soon. Thank you very much again!
p
Hi @faint-sandwich-26722 Will do :)
❤️ 1
f
Hey! Here it is: https://github.com/ory/elements/pull/54 Another question: I have two errors, here dans here
Copy code
This condition will always return 'false' since the types 'UiNodeGroupEnum' and '"saml"' have no overlap
This enum seems to be defined in @ory\client\api.ts. So should I do another PR in order to update it?
p
Thank you! I will check it out soon. Regarding the UINodeGroupEnum - I don't see SAML there on the latest
@ory/client
. it's possible we just need to create a new release for it.
f
Thank you :) Yes there is no SAML yet on
@ory/client
yet as, as far as I know, the SAML federation concept only exists from out contributions. So should we do something on
@ory/client
too, or wait for you to add the enum value in another release?
p
Ah i see, yeah this will probably need to be added in Kratos, probably under your SAML PR https://github.com/ory/kratos/pull/2653 https://github.com/ory/kratos/blob/8406eaf92006d9812108bd3ae57245f01e627bfc/ui/node/node.go#L39-L51 I guess we could also just add the saml group manually for now through a type union inside Elements and then later remove it once the sdk supports it.
f
Yes we've already done that 🙂 Here: https://github.com/ovh/kratos/blob/saml/ui/node/node.go#L42 I think we did all we needed to do in Kratos for the ui part. Okay let's do this then. Thank you!