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

Generic use enhancement request #51

Open
hschaefer123 opened this issue May 9, 2024 · 0 comments
Open

Generic use enhancement request #51

hschaefer123 opened this issue May 9, 2024 · 0 comments

Comments

@hschaefer123
Copy link

Hi,

i am currently implementing the SAPUI5 ushell with CAP (similar to WorkZone).
This works like a charm, but i have to struggle with some hardcoded values.
Since the ushell and the notification service is designed to be agnostic it would be great, if this plugin can support other implementations as well.

  1. Problem with hardcoded endpoints

inside notifyToRest.jsthere is a hardcoded NOTIFICATIONS_API_ENDPOINT
(similar notificationTypes.js NOTIFICATION_TYPES_API_ENDPOINT).

This leads to Problems with CAP service endpoints with '.srv' postfix.

It would be great, if this can made customizeable inside package section (something like this)

notifications: {
  notificationsAPIEndpoint: '/odata/v4/notification',
  notificationTypesAPIEndpoint: '/odata/v4/notification',
}

Currently, i am using the following workaround to get it working for now:

// uri rewrite for harccoded @cap-js/cds-typer Work Zone api endpoint
const originPath = '/odata/v2/Notification.svc'
//@ts-ignore
cds.app.post(`${originPath}/*`, function (req, _res, next) {
    req.url = req.url.replace(originPath, '/odata/v4/notification')
    next()
})
  1. Forced usage of CSRF Tokens.

Running locally, CAP does not support CSRF Token for POST requests.
It would be great, if you can make this more flexibel.

There is an easy solution for this in notifyToRest.js

await executeHttpRequest(notificationDestination, {
    url: `${NOTIFICATIONS_API_ENDPOINT}/Notifications`,
    method: "post",
    data: notificationData,
    headers: csrfHeaders,
  }, { fetchCsrfToken: notificationDestination.csrfProtection ?? true });

Using the fetchCsrfToken option against the local destination with fallback to true (like original).

Now you can use a default-env.json destination like this (support by approuter)

{
    "destinations": [
        {
            "name": "SAP_Notifications",
            "url": "http://localhost:4004/odata",
            "authentication": "BasicAuthentication",
            "username": "",
            "password": "",
            "csrfProtection": false
        }
    ]
}

Currently, i am using the following workaround to get it working for now:

// Generic CSRF Fake Handler to allow local usage of preflight
//@ts-ignore 
cds.app.head('*', function (req, res, next) {
    const mode = req.get('X-CSRF-Token')
    if (mode && mode.toLowerCase() === 'fetch') {
        res.set({
            'X-CSRF-Token': 'dummy'
        }).status(200).end()
    } else {
        next()
    }
})
  1. Usage with profile development
    The types are handled hardly different for !production. Would be great to get this controlable

Also workaround is nessessary to get it running as expected with profile dev using default-env.json destination:

notifications: {
  kind: "notify-to-rest"
  outbox: false
}

It would be great, if you can add this enhancements to your code.

Regards
Holger

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

No branches or pull requests

1 participant