pleszr / motivaa

MIT License
1 stars 1 forks source link

Keycloak backend frontend integration #10

Closed csabika98 closed 4 months ago

csabika98 commented 5 months ago

BACKEND:

habito-backend/habito-app/pom.xml ->

Added

org.springframework.security spring-security-oauth2-jose org.springframework.boot spring-boot-starter-oauth2-resource-server org.springframework.boot spring-boot-starter-oauth2-client

Created a new .properties file classpath:motivaa.properties -> for storing keycloak data independently

habito-backend/habito-app/src/main/java/com/habito/boundary/HealthCheckController.java Added PreAuthorize -> Making sure you can only access to if you are logged in to Keycloak

habito-backend/habito-app/src/main/java/com/habito/config/SecurityConfig.java

Very straightforward -> created new package, config -> SecurityConfig is set to ACCEPT TOKENS FROM KEYCLOAK, and YOU CAN CONTROL WHICH ENDPOINTS CAN BE PUBLIC AND WHICH ARE NOT

FRONTEND:

habito-frontend/src/App.js

Making sure we are passing the tokens to the frontend endpoint

habito-frontend/src/index.js

Added .js extension, as there were an issue when i tried to run React with NODE 21 LTS

csabika98 commented 5 months ago

Hello @pleszr !

The following changes have been applied:

-> motivaa.properties removed, all the entries now its in the application-local.yaml -> package.json, proxy entry removed, .env.template created, where we can customize the PORT

csabika98 commented 5 months ago

Hello @pleszr !

i have added new test cases to test the backend Keycloak int.

csabika98 commented 5 months ago

Hello, @pleszr

Detailed Changes

src/components/UserInfo.test.js: Unit tests to verify the UserInfo component’s behavior based on different authentication states.

src/App.test.js: Comprehensive tests to validate the App component’s behavior during different stages of data fetching and error handling.

GitHub Actions Workflow (frontend-tests.yml): A CI/CD workflow to automate the testing of frontend components on each commit and pull request

Tests: Backend tests now include scenarios for both authenticated and unauthenticated requests to verify the security measures.

csabika98 commented 4 months ago

Question: For setup():

I had to define it at first then, i figured out its enough if we have @AutoConfigureMockMvc, but i kept it and just commented it, initially when, we are mocking or mimicing the way we would expect the App behaves in certain scenario had to define this setup()

It can be removed, its just a reminder that no need to define manually setup() anymore.. as now.. we have @AutoConfigureMockMvc ( when i last used Spring in CodeCool i used setup() i was not aware of this)

The @AutoConfigureMockMvc annotation automatically configures and injects a MockMvc instance for us.


Question: For: i think this is redundant due to .andExpect(status().isUnauthorized()) ?

I see, i made a mistake here, well, not a mistake but its not following the clean code principal but we dont need assertEquals(401, status); because andExpect already performs this check.

In a test, using andExpect(status().isUnauthorized()) is effectively an assertion.

so andExpect(status().isUnauthorized()) == assertEquals(401, status);

@test void shouldDenyUnauthenticatedUserAccessToAuthTest() throws Exception { mockMvc.perform(get("/apis/authTest")) .andExpect(status().isUnauthorized()); }


Question: **For:

I dont think this is coherent.

if we are expecting a response should we not assert it? the printing is not in sync with the test itself**

I understand your concern.

Because the andExpect methods are assertions, The test includes assertions for the status (isOk()) and content type (application/json)

Therefore no need to use assertEquals..

Additionally, printing expected vs. actual values can be useful for debugging, my mistake i was not sure about the expectation of this endpoint thats why i wrote {} as expected response.. in here we are comparing responses.. what would be the expectation and what is being called by the mockMVC