Skip to content
This repository has been archived by the owner on Feb 13, 2024. It is now read-only.

Keycloak vcauthn login #565

Merged
merged 13 commits into from
Aug 12, 2021
Merged

Conversation

Jsyro
Copy link
Contributor

@Jsyro Jsyro commented Aug 5, 2021

New class which replaces micronauts default class if keycloak is being used.

New class is identical except for presentationRequestConfigurationId member and loading of that value into the parameters of the redirect request.

Requires micronaut.security.oauth2.clients.keycloak.vcauthn.pres_req_conf_id to be set to non-null value other application will not start. Is there a safe way to load this with a default. The ideal behaviour is that keycloak would be able to log in in any way but using the exact vc-authn service that handles the VC Verification would not work without the proper value set.

Looking for input, could possibly configure a more generic query parameter injection.

Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
@Jsyro
Copy link
Contributor Author

Jsyro commented Aug 5, 2021

screenshots of local environment using cloud hosted keycloak to authenticate with BPA (via BCGov vc-authn service).

image

image

image

return parameters;
}

}
Copy link
Contributor

Choose a reason for hiding this comment

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

How about:

import io.micronaut.context.annotation.Replaces;
import io.micronaut.context.annotation.Value;
import io.micronaut.http.MutableHttpResponse;
import io.micronaut.security.oauth2.endpoint.authorization.request.AuthorizationRequest;
import io.micronaut.security.oauth2.endpoint.authorization.request.DefaultAuthorizationRedirectHandler;
import lombok.extern.slf4j.Slf4j;
import org.hyperledger.bpa.security.oauth2.client.RequiresKeycloak;

import javax.inject.Singleton;
import java.util.Map;

@Slf4j
@Singleton
@RequiresKeycloak
@Replaces(DefaultAuthorizationRedirectHandler.class)
public class KeycloakAuthorizationRedirectHandler extends DefaultAuthorizationRedirectHandler {

    @Value("${micronaut.security.oauth2.clients.keycloak.vcauthn.pres_req_conf_id}")
    String presentationRequestConfigurationId;

    /**
     * {@inheritDoc}
     */
    @Override
    protected Map<String, Object> instantiateParameters(AuthorizationRequest authorizationRequest,
                                                        MutableHttpResponse response) {
        Map<String, Object> parameters = super.instantiateParameters(authorizationRequest, response);

        parameters.put("pres_req_conf_id",this.presentationRequestConfigurationId);
        log.info("keycloak vcauthn pres_req_conf_id = " + this.presentationRequestConfigurationId);

        return parameters;
    }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely, sorry i missed that.

Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
@@ -0,0 +1,182 @@
version: "3.9"
Copy link
Contributor

Choose a reason for hiding this comment

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

I see a couple of issues here:

  1. We want to reduce the number of compose files back to a single one as new users of the repo find it hard to find the right file.
  2. There is no explanation and link that this is related to vc authn. There is documentation for plain Keycloak: https://github.com/hyperledger-labs/business-partner-agent/blob/master/docs/concepts/security_keycloak.md but no example compose file, while here it is the other way round
  3. Without the knowledge on how to setup vc-authn just having this single file is only meaningful to you but everyone else has no clue what is going on.

So if this is necessary we need to find a better structure for this for example

-scripts
-- scenarios
--- plain-keykloak
--- keycloak-vc-authn

Another option would be to assume that all of this config happens somewhere else maybe in the helm chart repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added some readme and organised the code by use. .

Copy link
Contributor

Choose a reason for hiding this comment

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

Like we discussed on the last weekly meeting we plan to get rid of the many compose files and switch to compose profiles. As that many files cause a lot of confusion and are hard to maintain. So be aware that the files in your PR will get deleted again. But for now it's ok to merge them as they live in a sub folder.

Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
Signed-off-by: Jason Sy <jasyrotuck@gmail.com>
@Jsyro Jsyro merged commit a827d1d into hyperledger-labs:master Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants